feat(nav): redirect to manage if there's no dropdown on mobile - #4949
feat(nav): redirect to manage if there's no dropdown on mobile#4949Chew wants to merge 2 commits into
Conversation
|
My eye darts between the 3 Manage labels that all have a different semantic meaning: 
One is a button, another is a heading, and another is a link.
I think this is the solution we want. This should be doable with some conditional rendering in the Blade template. If there's nothing to show in the dropdown, we can use a simple link. |
|
Maybe I was looking in the wrong places, basically every role should have |
|
Yep. I would most certainly classify this issue as a bug 🐛 |
|
I'll take a look into that and see if we can resolve the root issue instead 😃 |
|
This is apparently not an issue with the tool selection since it works in development. Works on my machine™ strikes again. I'll rework this to redirect if there's nothing in the dropdown instead. |
Chew
changed the title
| dropdown-class="dropdown-menu-right" | ||
| :title="__('Manage')" | ||
| :desktopHref="route('filament.admin.pages.dashboard')" | ||
| :force-href="!$anyDropdownItems" |
There was a problem hiding this comment.
nit: Better to call this something like shouldForceHref. At the moment, it's almost indistinguishable in purpose between desktopHref unless the reader looks around to see what the type of $anyDropdownItems is.
Might be a good idea to rename $anyDropdownItems too to something like $hasAnyDropdownItems.
| $user?->can('develop') || | ||
| $user?->can('manage', App\Models\Ticket::class) || | ||
| $user?->can('manage', App\Models\AchievementSetClaim::class) || | ||
| $user?->can('manage', App\Models\GameHash::class) || |
There was a problem hiding this comment.
Minor: For users whose permissions pass these checks but are still blocked by the later nesting, $anyDropdownItems will technically become true even though no item is rendered.
For example, a Junior Developer can manage tickets/claims per the policy classes, but they fail the outer @can('develop') on line 41. Therefore, assuming I have my boolean math correct, the mobile Manage trigger still renders as a non-link button with an empty dropdown instead of forcing the Filament dashboard href.
2 participants
https://canary.discord.com/channels/310192285306454017/1498889559998595144/1498889559998595144
This PR allows mobile users who see no dropdown a way to get to the manage area.
It does (a bunch of) checks to see if there's a visible dropdown to show, and if there is not, just redirect the user.
All of this only affects mobile users. As a result, an override of the desktopHref was needed to let us effectively make it act like it does in desktop on mobile if there's no dropdown to show.