- ðŸ‡ðŸ‡ºHungary Gábor Hojtsy Hungary
@bnjmnm pinged me for product management feedback. Given that https://www.drupal.org/project/admin_toolbar → is the 4th most installed module, I think it would make sense to add it to core. Its a universal improvement. There was lots of resistence to add it to Drupal 7 when the current toolbar was added I think because the popup menus would have caused confusing with the overlay also popping up. With the overlay gone, I don't think that argument stands anymore.
That said, it would be great to somehow figure out which parts of the contrib module are 80%, as in including them in core would essentially serve as a replacement and remove the use case to install the separate contrib module for most sites. That would be a genuine improvement. If we are only adding part of the module but most sites would still install the contrib module then IMHO we would not move the needle too much.
Finally there are some high profile contrib projects that would be good to coordinate with, such as Gin Toolbar, so that they are not caught by surprise and/or improvements they need may be taken into account as well.
- First commit to issue fork.
- @bnjmnm opened merge request.
- 🇺🇸United States bnjmnm Ann Arbor, MI
Added the admin_toolcore branch that works with admin themes, and makes the menu depths configurable.
Also, like you observed and @NancyDru as early as November 2018 (#48) most of the deeper links are missing, such as display modes, but also Content Types > Article (missing) > Manage fields (missing) and Content > Add Content (missing) > Article (missing).
These links don't exist in the Adminstration Menu by default, thus they are not available in the updated toolbar here. There is a submodule of Admin Toolbar
admin_toolbar_tools
that generate these links, but I don't think replicating that should be in this issue's scope. This issue is introducing many beneficial UI changes already. Attempting to create NEW menu items within this issue's scope introduces it's own complexity and concerns . Those additional links can be added in a followup.Still needs test coverage as mentioned in #65. It may be possible to port some of the tests from Admin Toolbar.
- last update
over 1 year ago 29,370 pass - last update
over 1 year ago 29,373 pass - Status changed to Needs review
over 1 year ago 4:54pm 1 May 2023 - 🇫🇮Finland lauriii Finland
Tested this manually. I think we need to make some minor design improvements to how we indicate links with subitems, as well as focus styles. On top of that, we probably should try to address #6 because right now navigating the toolbar using keyboard is certainly more tedious than it used to be.
- last update
over 1 year ago 29,383 pass - last update
over 1 year ago 29,385 pass - 🇫🇮Finland lauriii Finland
Made some minor changes to the styles in collaboration with @ckrina.
- last update
over 1 year ago 29,386 pass - Status changed to Needs work
over 1 year ago 5:53pm 10 May 2023 - 🇫🇮Finland lauriii Finland
I don’t this we need to provide an option in core to limit the menu depths. This could be part of the contrib module functionality. There are benefits to keeping the experience consistent across sites by limiting the configuration options people have on things that are not critical.
The main reason I’m thinking we should remove it is that there are efforts to redesign the toolbar and I don’t know if this feature will make sense anymore after that. From that perspective, adding it will add more complexity.
Also, this doesn’t change the configuration for the vertical toolbar, and it's not something we have provided as a configuration there. In future if this is deemed as something we need, we could add it separately as its own feature.
- last update
over 1 year ago 29,400 pass - Status changed to Needs review
over 1 year ago 2:46pm 30 May 2023 - 🇺🇸United States bnjmnm Ann Arbor, MI
I agree with #107 - it was my personal preference anyway but I was trying to go with an approach that is more likely to land. Since a PM is making the suggestion I think this seems like a choice that won't obstruct things 🙂. The menu depth stuff has been removed from the MR
- Status changed to Needs work
over 1 year ago 11:22pm 30 May 2023 - 🇺🇸United States smustgrave
Seems there are some issues in the MR possibly from a rebase?
Maybe we could get a 11.x branch also?
- Open on Drupal.org →Environment: PHP 8.2 & MySQL 8last update
over 1 year ago Not currently mergeable. - last update
over 1 year ago 29,446 pass - last update
over 1 year ago 29,446 pass Note that accessible keyboard navigation is being added to the admin_toolbar module in this issue: https://www.drupal.org/project/admin_toolbar/issues/3286466 ✨ Tabbing order does not satisfy 508 accessibility requirements Needs review
Maybe the same pattern can be used here
I'm testing 3821 (in Firefox) and had a few issues:
1. If I set the menu to vertical orientation, then back to horizontal, the arrow icons disappear and there is no more focus state
2. I find it unintuitive that the vertical and horizontal modes use different keyboard navigation patterns. I also wasn't able to navigate the horizontal toolbar using my assistive software (Vimium browser addon which allows you to access any interactive element on the page using a 2-letter sequence) because the chevrons in the horizontal mode are not a separate element that open/closes the menus (the way they are in vertical mode).
Though not every keyboard-only user uses Vimium, this kind of functionality is pretty common in assistive software (for example Voice Control, ShortCat and VimMotion provide this functionality for macOs). Having the collapse/expand button be its own element would make it easier in general for assistive software to interact with.
For this reason I think the chevrons should be their own element and allow keyboard users to open/close the associated menu by pressing "Enter" or "Space" while under focus. This could be implemented in addition to the "arrow keys" functionality (and in general I think it's a good accessibility principle to provide multiple ways to do the same thing)