Menu links should have id attribute

The toolbar_tools_menu_navigation_links() function in this module is clearly based on toolbar_menu_navigation_links() in the core toolbar module. I am not sure why the current version (in this module) does not set the id attribute. Perhaps it is intentional, for performance reasons.

I checked the Git history for core:

% git hist -S "['attributes']['id']" -- core/modules/toolbar/toolbar.module 
...
* 70bed3385fe 2014-07-30 | Issue #2301317 by pwolanin, dawehner, Wim Leers, effulgentsia, YesCT, xjm, alexpott: MenuLinkNG part4: Conversion. [Alex Pott]
* 44f76c6bcff 2014-07-30 | Revert "Issue #2301317 by pwolanin, effulgentsia, Wim Leers, dawehner, alexpott: MenuLinkNG part4: Conversion." [Alex Pott]
* fd2db9cd352 2014-07-30 | Issue #2301317 by pwolanin, effulgentsia, Wim Leers, dawehner, alexpott: MenuLinkNG part4: Conversion. [Nathaniel Catchpole]
...
* ab07a33aba4 2013-02-01 | Issue #1847198 by benjifisher, jessebeach, effulgentsia: Update the structure returned by hook_toolbar(). [webchick]
...
* 7bbf113d4ed 2012-11-21 | Issue #1137920 by jessebeach, lewisnyman, tkoleary, Bojhan, webchick, benjifisher, nod_, sjbassett, kathryn531, effulgentsia, Everett Zufelt: fixed toolbar on small screen sizes and redesign toolbar for desktop. [Dries]

So the id attribute is set in the core module at least since 2014. (The commits from 2012 and 2013 are some of my earliest contributions to Drupal.)

A similar search in this module showed no results, so it seems the id attribute has never been set:

% git hist -S "['attributes']['id']" -- admin_toolbar.module && echo done
done

The tests are now passing, so MR !166 is really ready for review. (Well, eslint complains. But I did not touch any JavaScript files, and that test also has warnings on the 3.x branch.)

Thanks a lot @benjifisher for raising this issue and getting the work started on this!

Just added a quick comment to the merge request, could you please take a quick look when you have some time and let us know what you think?

Thanks in advance!

Today's comments on 📌 Use ARIA disclosure pattern for submenu buttons in vertical toolbar orientation Needs work suggest using something other than the id attribute (and using text inside the button instead of an aria-lebelledby attribute). So maybe wait on this issue until we get a decision there.

Thanks Benjin (@benjifisher) for letting us know.

We'll wait to see what happens in the related issue before moving forward with the suggested changes.

Thanks again!