Drupal admin_toolbar disable sticky/fixed state. Is there any option to do this?

Yes, I also prefer to not show redundant and distracting elements, and maximize the working area, when I am creating content, so this would be a nice feature.

Here's a rough sketch. For some reason, the boolean value is saved as 1 or 0, and not true or false ... so, if for some reason it is saved as 1 on some systems and true on others, used this:

if ($toolbar_top === TRUE || $toolbar_top === 1)

I created an issue about the 1/true thing.

silvi.addweb → made their first commit to this issue’s fork.

I have added toolbar top to schema, Now boolean value is saved as true or false. please review.

Thank you very much for a fast reply, and fixing this problem for me @silvi.addweb. It now works perfectly, and the values used are true or false as desired. I have updated the check to just use if ($toolbar_top === TRUE). If you have time, feel free to review the patch, and see if it works as expected, and the rest of the code looks all right. Thanks!

I have applied the patch for this and it works as expected. Many thanks!

Thanks for reviewing @bmango! Feel free to change the status to RTBC.

Tested MR 76 on Drupal 10
The MR is applied cleanly...

Adding screenshot

RTBC+1

Rebased MR and fixed an issue with stylelint so all the jobs would pass ✅

Personal comments on MR!76:

1 - Could we please change the name of the config variable toolbar_top to disable_sticky?

Would anybody mind if I made the changes in the current MR?
Would anyone have a better suggestion for a better config name, or would you perhaps prefer keeping the current name?

I personally feel disable_sticky would be more explicit and easier to understand when reading the code.

2 - Additionally, I would recommend updating the wording of the added field on the settings form:
https://git.drupalcode.org/project/admin_toolbar/-/merge_requests/76/dif...
Toolbar at top ==> Disable sticky toolbar
Make toolbar stay at the top when scrolling. ==> Disable Admin Toolbar's sticky behavior so it stays at the top of the page when scrolling.

3 - Lastly, if possible, it would be great if this new setting could be automatically tested a minimum, building on top of the AdminToolbarSettingsFormTest class added in 🐛 [AdminToolbarSettingsForm] Fix Fatal Error: Call to undefined method Drupal\Core\Menu\MenuLinkManager::invalidateAll() Fixed (see MR!94): Check the CSS file is properly added to the rendered HTML markup if the setting is selected (TRUE), for example.

Otherwise, I've tested this manually from a functional standpoint and it's working fine: A very nice addition indeed.

Any feedback, comments and replies on the changes suggested would be greatly appreciated.
Thanks in advance!

Thank you @japerry for updating the MR, and all the other recent work on the module, and thanks @DYdave for the update here, and all the other great work you have done getting Admin Toolbar Drupal 11 ready. I very much appreciate it!

About your suggestions, they are great and I agree with all of them, so feel free to go ahead. Maybe someone can add the test you suggest?

Super nice and speedy reply on this @ressa! 🙂
As always, constructive, positive and very encouraging!

I've been following quite a lot of tickets on which you've been doing amazing work, coordinating development and helping developers make the right decisions, just like in this ticket.
It definitely feels great knowing you're keeping an eye on this one 👍

I'll get cracking on the changes later today, after work: They are pretty straight forward.

However, for the tests, I'm not sure exactly how to proceed, since the AdminToolbarSettingsForm is currently a major blocker, crashing on submit with a WSOD in the DEV branch as of release 3.5.0...

🐛 [AdminToolbarSettingsForm] Fix Fatal Error: Call to undefined method Drupal\Core\Menu\MenuLinkManager::invalidateAll() Fixed should be pretty much ready to get merged... and would greatly appreciate if Jakob (@japerry), you could please try taking a look at this one in priority.

Let's see how far we can take this ticket until the AdminToolbarSettingsFormTest class gets added to the dev branch.

Thanks again!

Quick follow-up on this issue:

The changes mentioned above at #17 have been implemented in MR!76 and are ready to be reviewed once again.

Once AdminToolbarSettingsFormTest class from MR!94 is added to the code base, we should be able to add disable_sticky => TRUE in the submitted form values and test:

• $assert->responseNotContains('css/admin_toolbar.disable_sticky.css') before and
• $assert->responseContains('css/admin_toolbar.disable_sticky.css') after.

 
For the time being, issue could probably be moved back to Needs review as an attempt to get more testing, feedback and reviews.

Thanks in advance!

Thank you @DYdave, it's great to hear! I try my best to help out where I can -- though I have to leave the hard core coding to the experts, such as yourself and @japerry 🙂

Hi Jakob (@japerry),

This ticket should also be ready to go.
I've just added basic checks (as suggested above at #20 for the added disable_sticky setting to the recently added AdminToolbarSettingsFormTest.

Merge request MR!76 has also been rebased with the latest changes to 3.x and still seems to be passing all tests.

Please let us know if you would see anything else that would still need to be addressed in order to get this feature added to the module, we would surely be glad to help.
Thanks in advance!

Hi, this is my first contribution and tested UI functionality. here is the screenshot.