Handle changes after Name change of menu. Can break sites if a user changed the name without noticing the handle change

[ralphmorris]

Hey,

Love this package, super useful and extendable! Thanks for your work on it.

I've noticed that when setting a Name for a navigation, the Handle is set automatically, which is slick. But if a user was coming back and updating the name of a navigation, the Handle is still updated again even if it has been specifically set to something.

Example use case: Footer menus

A common footer layout is 3 or 4 columns with a menu in each column. The handles could be named something to be looked up like footer_menu_1, footer_menu_2, footer_menu_3, footer_menu_4.

However, the Names for the fields might be something like Support, About us, etc. If an unsuspecting admin were to want to change the Name of a footer menu, they would quite likely also accidentally update the Handle which would stop the menu showing entirely.

Possible solutions include making the Handle field readonly when editing the menu, or not doing the auto update of the Handle if the Handle already has a value. Similar examples to this are on the Filament website when dealing with slugs here https://v2.filamentphp.com/tricks/generate-slugs-without-overriding

Interested to know your thoughts.

Cheers!

Ralph

[ryangjchandler]

Yep, this is a bug and should be fixed. I don't think I've ever actually changed the name of a menu so wouldn't have noticed this 😆

Auto-updating on create but not on edit is the best approach I think. There are definitely valid usecases for changing the handle, but it shouldn't be done automatically.

[ralphmorris]

Ha! Thanks Ryan, appreciated. It took me a while to spot it too and I've been working on navigations all week :)

I've just had a little play and seems a pretty simple fix:

TextInput::make('name')
    ->label(__('filament-navigation::filament-navigation.attributes.name'))
    ->reactive()
    ->debounce()
    ->afterStateUpdated(function (?string $state, Set $set, string $context) {
        if (! $state) {
            return;
        }

        if ($context == 'create') {
            $set('handle', Str::slug($state));
        }
    })
    ->required(),

I've added the $context variable to be passed in and wrapped the set in an if.

Let me know if you want this as a PR or you would prefer to put it in yourself.

Thanks again!

[ryangjchandler]

Feel free to open a PR @ralphmorris :)

[ralphmorris]

@ryangjchandler Done!

https://github.com/ryangjchandler/filament-navigation/pull/108

[ralphmorris]

Hey @ryangjchandler

Just wondering if you've had a chance to review this? Sorry, I'm sure you're busy!

Cheers

Ralph