[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