Add local tasks (tabs) for Admin Toolbar Settings forms

Quick follow-up on this issue:

Created initial merge request MR !139 above at #2 which adds local tasks (tabs) for Admin Toolbar Settings forms.

Moving issue to Needs review, as an attempt to get some feedback on this new Feature.

This MR "could" need some Tests for the tabs in a new test class for the admin_toolbar_search Settings form.
We could use an xpath expression for example to check for the tabs on the page (?!).

Any testing/reporting feedback, questions or suggestions would be greatly appreciated.
Thanks in advance!

I'll review this.

Hi @dydave, I've applied the provided MR as a patch, and it applied cleanly without any errors. Previously, as mentioned in the issue summary there was no way to navigate to the "Admin toolbar tools, Admin toolbar search and Admin toolbar" settings however the module configuration can be access under the menu "Configuration > User Interface > Admin Toolbar Search, Configuration > User Interface > Admin Toolbar Tools, Configuration > User Interface > Admin Toolbar".

After applying the patch, new tabs have been enabled to access/configure Admin toolbar, toolbar tools and toolbar search under Admin Toolbar Settings, i.e., /admin/config/user-interface/admin-toolbar. This is a great feature enabling users to easily switch between these sub-module settings by just switching the tabs. I've reviewed the changes and the changes are working fine.

I've reviewed the MR changes and dropped a suggestion regarding the title changes for Admin toolbar settings. Additionally, noticed one difference in the setting page title for Admin toolbar search config page. For Admin toolbar and tools config page, the page title has some inconsistency. For Admin toolbar setting config page title is 'Admin Toolbar settings', for tools it is 'Admin Toolbar Tools settings', however, for search it is 'Admin toolbar search settings'. As per my suggestion, the config page title for Admin Toolbar Search should be 'Admin Toolbar Search settings' to maintain the consistency for the modules. I've attached a screenshot for the changes to be made for Admin toolbar search config page title.

I'm keeping the issue status to Needs review for now, although the changes are working fine. If the suggested changes can be applied then we can change status to Needs work or else it be changed to RTBC.

I'm attaching the screenshots of before and after the changes of the applied MR as a patch for reference.

Thanks!

Thanks a lot Tirupati (@tirupati_singh) for the detailed review, it's greatly appreciated! 🙂

I have just updated the merge request with the following changes:

• Updated the Admin Toolbar Settings local task label, as suggested, from Settings to Toolbar settings.
• Added PHPUNIT Functional Tests to check whether the local tasks are displayed correctly.

 
I'm still wondering if it is a good idea to add tabs like that... 🤔
 
I mean, personally, I find these tabs super useful to easily switch between the forms while testing and developing on the module.
So with your positive feedback and confirmation the changes worked fine, I think we're probably going to include this new feature in the next 3.6.0 release 👌
 
In this case, we need to try to get a bit more feedback on the local tasks labels and this feature in general, so I'm going to add this issue to the 📌 [Meta] Roadplan for Admin Toolbar 3.6 Active , as an attempt to attract more attention from contributors and hopefully get more comments on this issue.
 
Feel free to give the MR another round of review and let us know if you spot anything else.
Thanks in advance!

Thanks @dydave and @tirupati_singh for creating, reviewing and improving tabs for Admin Toolbar!

(I was never sure what "local tasks" meant in Drupal -- for example in Views, "Tabs" is used, which is universally understood as being the same as a tab in a spreadsheet in Calc/Excel.)

Anyway, naming things like these menu tabs is not easy, since there is a main module, and submodules ... But I think you both reached a good balance, and the page and tab titles make sense intuitively, which is the task here.

And I agree very much with you @dydave, adding tabs is a great idea, which makes navigating between these naturally, closely related modules super easy.

Another GUI thing that I planned on addressing at some point with an issue, was the order of the three modules under "User interface", which has annoyed me for a long time. Perhaps we can take care of it, now that the UI is being worked on?

I have updated the MR, and added weight to the three modules, so that the order is more hierarchic, with "Admin Toolbar" at the top.

Before:

• Admin Toolbar Search
• Admin Toolbar Tools
• Admin Toolbar
• Shortcuts

Better?

• Admin Toolbar
• Admin Toolbar Search
• Admin Toolbar Tools
• Shortcuts

What do you think about this?