- Issue created by @alison
- ๐ฎ๐ณIndia dev2.addweb
nilesh.addweb โ made their first commit to this issueโs fork.
- Merge request !7Add support to restrict BigMenu usage to selected menus. โ (Open) created by Unnamed author
- Status changed to Needs review
9 days ago 5:41am 19 September 2024 - ๐ฎ๐ณIndia dev2.addweb
Hi, I have added config field to allow certain menus and created MR. Please review it.
- ๐บ๐ธUnited States alison
@nilesh.addweb Thank you! When I apply the patch, though, I no longer have Big Menu functionality on any of my menus -- even after enabling those menus in the Big Menu settings form. I'm not sure why, I don't see any errors in logs, it's just not functioning for me -- did it work for you?
Besides that, a few other notes:
- The first time I go to the settings form, I get a warning in logs (โฌ๏ธ). I think it'll be happier if you change the default value (in bigmenu.settings.yml) to
{ }
.
Warning: foreach() argument must be of type array|object, null given in Drupal\bigmenu\BigMenuForm->buildOverviewForm() (line 62 of /app/web/modules/contrib/bigmenu/src/BigMenuForm.php)
- As the patch is currently written, Big Menu is immediately "disabled" on all menus -- site admins have to go to the settings screen and enable menus, in order for Big Menu to keep working.
- Either there should be an update hook to enable Big Menu on all menus; or,
- The setting could be revised so that this option is a "negative" option, i.e. checking a box next to a menu would disable it (example: "disable_themes" option in the Chosen module, but this would be menus, of course) (this field in the Chosen module settings is also an example of the checkbox field type); or
- Allow either way: list the menus with checkboxes next to each one, and then have a "negate this condition" option, like in Block visibility settings. (This option would also need an update hook.)
- (what I'm referring to) In Block visibility settings > Vocabulary, there's a "negate this condition" checkbox.
- I think checkboxes would be better UX for this field, and more consistent with similar fields on Drupal admin forms (instead of a select field).
- I think the language would be better as "enabled menus" rather than "available menus", what do you think? (Or "disabled," if you go with the "negative" option.
- The first time I go to the settings form, I get a warning in logs (โฌ๏ธ). I think it'll be happier if you change the default value (in bigmenu.settings.yml) to