Make menu depth configurable

I will try to do it as a config form

I have done it.

You can test it.

I'm trying to apply the patch at the root of the module folder in the version 3.x-dev with no success.

Yes, unfortunately, that patch was generated incorrectly.
I recreated the patch.
Can you test it?

Patch applies cleanly now, works perfectly!

Add video as evidence.

Thanks for the work. My only concern is that we already have a config form in the Admin Toolbar Extra Tools module. So maybe we should use it.

Changed the functionality to Admin Toolbar Tools config menu. Please review.

The patch in the comment #8 applies perfectly and runs as expected!

+1 RTBC

1. +++ b/admin_toolbar_tools/admin_toolbar_tools.install
   @@ -32,3 +32,13 @@ function admin_toolbar_tools_update_8202() {
   +  ->save(TRUE);
   +}
   \ No newline at end of file
   
   +++ b/src/Render/Element/AdminToolbar.php
   @@ -36,7 +36,13 @@ class AdminToolbar implements TrustedCallbackInterface {
   +      $max_depth = \Drupal::config('admin_toolbar_tools.settings')->get('menu_depth');
   

   This removes the newline from the file.

2. +++ b/src/Render/Element/AdminToolbar.php
   @@ -36,7 +36,13 @@ class AdminToolbar implements TrustedCallbackInterface {
   +    if (\Drupal::service('module_handler')->moduleExists('admin_toolbar_tools')) {
   +      $max_depth = \Drupal::config('admin_toolbar_tools.settings')->get('menu_depth');
   +    }
   +    else {
   +      $max_depth = 4;
   +    }
   

   This is a bit awkward. The configuration is clearly for the admin toolbar module. I'm not sure what the added complexity with the module check and the extra module adds other then additional overhead of the module check. Seems to me like Roman had the right approach in #5.

Also, this adds an update to set the value and schema for the config but doesn't set the default value in the config install. Seems like that would make new installs inconsistent and possibly broken.

Looking back, none of the problems from my previous review apply to #5. IMHO I think that was probably the right approach and probably RTBC.

One small suggestion: The config should be checked before updated in hook_update_N().
But as that was not done even in previous updates, then I am OK with that.

I agree with #11. Patch #8 needs some work.
Why is it a problem to create a new settings form?
IMHO it does not right to add this configuration in the admin_toolbar_tools module.

A new patch with the same approach of patch #5.

Ops.

Testing Steps

• Go to admin/config/user-interface/admin-toolbar
• Click in the dropdown Menu depth
• Change the value to 1
• Save
• Check the menu depth

Expected Results

• Change the menu depth accordingly to the value selected

Actual Results

• Menu depth correctly changes accordingly to the value selected

The code follows the standards and not change any other functionalities.

Thanks