[wp-trac] [WordPress Trac] #47067: Twenty Nineteen: fix markup errors in `twentynineteen_add_ellipses_to_nav()`

[WordPress Trac]

#47067: Twenty Nineteen: fix markup errors in
`twentynineteen_add_ellipses_to_nav()`
------------------------------+---------------------
 Reporter:  afercia           |       Owner:  (none)
     Type:  defect (bug)      |      Status:  new
 Priority:  normal            |   Milestone:  5.3
Component:  Bundled Theme     |     Version:
 Severity:  normal            |  Resolution:
 Keywords:  has-patch commit  |     Focuses:
------------------------------+---------------------
Changes (by ianbelanger):

 * keywords:  has-patch needs-testing => has-patch commit


Comment:

 Just tested [attachment:"47067.diff"] and it still applies cleanly. It
 does take care of issues 1 & 2. However, I am not sure that issue 3, is
 really an issue at all.

 > `<li id="menu-item--1"`: not sure why there's this hardcoded ID: it
 produces duplicate IDs in a page, which is invalid HTML and I don't see a
 good reason to use it in the first place, unless I'm missing something

 I would agree that hardcoding an `id` into a `menu-item` is the wrong
 approach. However, this menu item is for the "Back" button, when the
 ellipsis has been clicked and the `sub-menu` is showing. As far as I can
 tell, it is not used anywhere else on the page, even when all menu
 locations are used. Also it uses 2 hyphens, where other menu items use
 just 1. `menu-item--1` vs `menu-item-1` Therefor, there shouldn't be any
 duplicate IDs on the page.

 All that being said, the `id` for this `menu-item` is unnecessary. I could
 not find any `js` or `css` in the theme that targets this particular `id`
 or the corresponding `class`, so I will be uploading a patch that removes
 both of them. I will leave it up to the committer to choose which approach
 to use.

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