[wp-trac] [WordPress Trac] #38023: Menus screen: simplify the Menu Settings controls

WordPress Trac noreply at wordpress.org
Tue Oct 25 07:34:16 UTC 2016


#38023: Menus screen: simplify the Menu Settings controls
---------------------------------------+--------------------------------
 Reporter:  afercia                    |       Owner:  rianrietveld
     Type:  defect (bug)               |      Status:  assigned
 Priority:  normal                     |   Milestone:  4.7
Component:  Menus                      |     Version:
 Severity:  normal                     |  Resolution:
 Keywords:  has-screenshots has-patch  |     Focuses:  ui, accessibility
---------------------------------------+--------------------------------

Comment (by GaryJ):

 Replying to [comment:10 rianrietveld]:
 > @GaryJ can you please specify more what you mean?
 > The CSS/classes are quite similar of what they where.

 That doesn't mean they were right first time :-)

 The patch here is an example of why using element selectors is not great -
 a change in the choice of element should not necessarily mean a change in
 the CSS. If CSS classes are used, these coupled changes are not needed (as
 much).

 If the original `dl` had got a class of something like `menu-settings-
 item`, then the `dl` could have changed to `fieldset` without any
 corresponding CSS change (the extra styles for `legend` would have been
 needed).

 My suggestion is to add a class like that in now, and attach the CSS to
 it, so that any future markup changes don't necessarily need to change the
 CSS as well.

 Being able to use a class selector, not only decouples the elements from
 the CSS, it also means the CSS can have reduced specificity, as `.menu-
 settings-item {}` could probably be used, instead of `.menu-settings
 fieldset {}`.

 My suggestion isn't a critical blocker, and the current patch would work
 as is, but if there's an opportunity to use better practices, and leave
 the code slightly cleaner than before the patch, I'd personally rather see
 it done.

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


More information about the wp-trac mailing list