Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Recently, there were a few additions and changes to the Admin Toolbar settings page, and some elements could need a little adjustment.
Menu depth position
When accessing the Admin Toolbar settings page, the "Menu depth" setting is placed at the top, though it's probably fairly rare that it needs adjustment, except for those installations with a huge number of modules installed.
Also, the new features "Toolbar sticky behavior" and "Toolbar hoverIntent behavior" settings are probably more likely to be changed.
Toolbar sticky behavior options
We could rename and place the Toolbar sticky behavior option "Disabled, show on scroll-up" between "Enabled" and "Disabled", since it is an "in-between state" after all.
The wording could also get tightened up, to simply "Disabled, show on scroll-up".
Steps to reproduce
Proposed resolution
- Place the "Menu depth" setting last on the Admin Toolbar settings page.
- Re-order and rename "Disabled, show on scroll-up"
Remaining tasks
Decide if those are good ideas.
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | after.png | 253.35 KB | rakesh.regar |
| #9 | before.png | 255.04 KB | rakesh.regar |
Issue fork admin_toolbar-3520680
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
ressaComment #4
ressaComment #5
schillerm commentedHi, I just reviewed and tested this locally on a D10.4.6 site. Everything works as expected and the new render arrays/ page layout make sense. Can't find any problems with spelling / words etc. +1 for RTBC
Comment #6
ressaThanks for reviewing @schillerm, feel free to change the Status :)
Comment #7
rakesh.regarRTBC+1 updating status.
Comment #8
ressaThanks, though it's not clear to me if you tried the patch and evaluated the changes?
I ask because I see that you commented "Patch in #9 works fine, this has been verified by multiple developers, moving this to RTBC." in #3097962: PHP Fatal error: Declaration of Drupal\\flag_anon\\FlagAnonLinkBuilder::build($entity_type_id, $entity_id, $flag_id) ...
Changing status to RTBC requires that you apply the patch, and check if the changes make sense:
From https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-...
Comment #9
rakesh.regarThanks for the feedback!
I understand that I should have added more details before moving this to the RTBC.
I have reviewed the MR and tested on my Drupal 10.4.6 installation and here are the details of my testing:
Patch application: I downloaded patch from the MR and applied using git apply.
Testing:
Before applying the patch -
After applying the patch -
Additionally:
I verified that there are no coding standard issues (phpcs) introduced by these changes.
Also no new errors or warnings were observed in the logs.
Based on this review, I believe the changes in MR can be merged.
Thank you!
Comment #11
dydave commented@ressa! 🥳
Thanks a lot for your great help keeping this show on the road!
I've been very busy with work on projects lately and couldn't find the time to reply earlier 😅
OK, I've taken a quick look at the merge request and it looks great!👍
While we're at it, I thought we could go a bit further and added the following two changes:
I didn't really "like" to see the "Menu depth" field "floating" in the form like that...
Additionally, based on your comments above @ressa, I assumed we could probably hide the setting by default, since it is not "very" likely to be changed.
As for all the wording, labels, texts, etc... these are more suggestions, at this point:
Feel free to edit any added textual elements as you would see fit 🙂
Overall, these changes would help us pretty much stabilize/wrap up all the previous changes made to this form, so they could be properly packaged in the next stable release 👍
I would greatly appreciate any feedback and reviews on the changes to the form (collapsed/open?), the help text, or anything else we could have missed.
Thanks again everyone for the help moving this issue forward! 🙏
Comment #12
ressaHi @dydave, great to have you back and moving the improvements along, awesome! I really like your suggestions for the expanded text descriptions, and they are perfect.
Actually, after making the MR and thinking more about it afterwards -- I also wasn't too happy about the "Menu depth" drop-down, and how it seemed to sort of get tacked on randomly at the end ... and, believe it or not, I even thought about perhaps putting the "Menu depth" drop-down under an "Advanced" fieldset, but then got away from it again. So great that you caught this detail, and actually did it 🙂
It looks ready to me, and very nice that we could get these final touches included, so let's move it along. It'll be great to get all the recent improvements released. Thanks!
Comment #14
dydave commentedThanks a lot @ressa for the prompt and positive feedback! 🥳
Really happy I'm able to look at the module again and keep working with you on the next release 🤝
Following your confirmation above at #12, I did a last review of the merge request and since all the jobs and Tests seemed to be passing 🟢, I went ahead and merged the changes above at #13 🥳
Very good work in this issue, following-up with the recent changes that were made to the settings form and the added Tabs 👍
This is really helping make the module more presentable in the next release.
Marking issue as Fixed for now:
Feel free to create more issues if you see any further improvements that could be made to the form or anything we could have missed.
Otherwise, we'll make sure the form stays a bit maintained when adding more parameters or making changes in the future.
Thanks again to everyone for the great help testing and reviewing this issue! 🙏
Comment #15
ressaFantastic @dydave, and I agree we have a great co-operation with the recent additions to Admin Toolbar 🙂
It's so great to get yet another improvement ready and committed 🎉
PS. @rakesh.regar: Thank you for the very thorough review you provided. I was positive that I posted a "Thank you, now you should totally feel free to change Status to RTBC" comment, but can't see it now ... so maybe I just wrote it, but forgot to post it? But here it is anyway: Thank you so much for the very thorough review in your comment #9! It was great to get confirmation that the patch worked as expected, and did what it was intended to do, and that you even checked for coding standard issues (phpcs). I am so sorry, that I forgot to post the comment.