[wp-trac] [WordPress Trac] #39692: Fix missing assignment of menus on theme switch

WordPress Trac noreply at wordpress.org
Mon Jun 26 20:53:57 UTC 2017


#39692: Fix missing assignment of menus on theme switch
-------------------------------------+-----------------------
 Reporter:  melchoyce                |       Owner:
     Type:  enhancement              |      Status:  assigned
 Priority:  normal                   |   Milestone:  4.8.1
Component:  Menus                    |     Version:
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch needs-testing  |     Focuses:
-------------------------------------+-----------------------

Comment (by joyously):

 Looking at the last patch, I think there are two cases that are strange
 for theme switch, and apparently this is not handling Customizer
 previewing at all.

 Case 1: the new theme has theme_mods already. Those should be used,
 untouched.
 Case 2: the old theme has no menu selected. Nothing to do for new theme.
 The old code (removed) handled these. The new code handles case 2, but not
 straightforward.

 Also, the new function is called `_wp_menus_changed()`, but it is about
 changing themes, not menus. Perhaps a little more clarity in the name, so
 it can be called from both `switch_theme()` and whatever the Customizer
 equivalent is?

 I also think the call to get the menu locations should be consistent. So
 in theme.php, instead of
 {{{
 $nav_menu_locations = get_theme_mod( 'nav_menu_locations' );
 }}}
 use the same call, like
 {{{
 $nav_menu_locations = get_nav_menu_locations();
 }}}

 And since we are talking about a switch, the temp option could be named
 more clearly. Instead of ` 'theme_switch_menu_locations'`, could it be `
 'theme_switch_old_menu_locations'` or `
 'theme_switch_from_menu_locations'`?

 I wondered about language. Since Weston shows the most used menu slugs,
 and they are all English, all the other slugs that didn't make the top 30
 list could include words in other languages along with less popular
 English words. I guess that doesn't matter.

 Having looked at over 400 themes for theme review, I might classify the
 common slugs this way:
 {{{
 $common_slug_groups = array(
   array( 'header', 'main', 'primary' ),
   array( 'secondary', 'subsidiary', 'top', 'custom' ),
   array( 'bottom', 'footer' ),
 );
 }}}

--
Ticket URL: <https://core.trac.wordpress.org/ticket/39692#comment:35>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list