Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Navigation Item Current Attribute dropped on dispatched event. #918

Closed
3 tasks done
kyledoesdev opened this issue Dec 22, 2024 · 14 comments · Fixed by livewire/livewire#9130
Closed
3 tasks done

Navigation Item Current Attribute dropped on dispatched event. #918

kyledoesdev opened this issue Dec 22, 2024 · 14 comments · Fixed by livewire/livewire#9130
Assignees

Comments

@kyledoesdev
Copy link

Flux version

1.1

Livewire version

3.5.17

What is the problem?

When an event is dispatched from a component to another component, the :current Attribute on a navigation item seems to drop off. Attached is a demo video of this happening.

2024-12-22.12-09-03.mp4

Code snippets

Attached are the files involved with this issue:

Navigation Badge Livewire Component

<?php

namespace App\Livewire\Invites;

use App\Models\Invite;
use Livewire\Attributes\Computed;
use Livewire\Attributes\On;
use Livewire\Component;

class NavigationBadge extends Component
{
    public function render()
    {
        return view('livewire.invites.navigation-badge');
    }

    #[Computed]
    #[On('invitation-updated')]
    public function invitations()
    {
        return Invite::getPending();
    }
}

Navigation Component Template

<div>
    <flux:navbar.item
        href="{{ route('invites') }}"
        :badge="$this->invitations"
        badge-color="sky"
        icon="envelope" 
        :current="request()->is('invites')"
    >
        Invitations
    </flux:navbar.item>
</div>

Model getPending query function.

public static function getPending(): int
  {
      return self::query()->where('user_id', auth()->id())->where('status', self::PENDING)->count();
  }

Navigation Layout Header.

<flux:header container>
    <flux:sidebar.toggle class="lg:hidden" icon="bars-2" inset="left" />

    <flux:navbar class="-mb-px max-lg:hidden">
        <flux:navbar.item icon="home" href="{{ route('welcome') }}" :current="request()->is('/')">
            Home
        </flux:navbar.item>

        @auth
            <flux:navbar.item icon="adjustments-horizontal" href="{{ route('dashboard') }}" :current="request()->is('dashboard')">
                Dashboard
            </flux:navbar.item>

            <livewire:invites.navigation-badge />
        @endauth

    </flux:navbar>

    <flux:spacer />

    <flux:button x-on:click="$flux.dark = ! $flux.dark" icon="moon" variant="subtle" aria-label="Toggle dark mode" />

    @auth
    
        <flux:dropdown position="top" align="start">
            <flux:profile avatar="{{ auth()->user()->avatar }}" />

            <flux:menu>
                <flux:menu.item icon="arrow-right-start-on-rectangle" wire:click="logout">
                    Logout
                </flux:menu.item>
            </flux:menu>
        </flux:dropdown>
    @else
        <flux:button size="sm" class="mx-1" href="{{ route('twitch.login') }}" icon-trailing="arrow-up-right">Login</flux:button>
    @endauth
</flux:header>

Invite approved function that dispatches event:

public function update($inviteId, $status)
  {
      $this->form->update($inviteId, $status);

      $this->dispatch('invitation-updated');
  }

How do you expect it to work?

I expect it to when the event is dispatched, the count of the badge in the navigation item to be updated without the theming underline disappearing. The current route did not change.

Please confirm (incomplete submissions will not be addressed)

  • I have provided easy and step-by-step instructions to reproduce the bug.
  • I have provided code samples as text and NOT images.
  • I understand my bug report will be closed if I haven't met the criteria above.
@jeffchown
Copy link

jeffchown commented Dec 22, 2024

@kyledoesdev That's happening because your navigation item is a Livewire component containing the event listener thus it is getting re-rendered when the event is dispatched and, at that point, the route is livewire/update so the navbar.item's route does not match.

I believe if you move the navbar.item markup outside of the component, and refactor the component to only render the flux:badge, all will work as you expect, e.g.:

layout-level:

<flux:navbar.item
    href="{{ route('invites') }}"
    icon="envelope" 
    :current="request()->is('invites')"
>
    Invitations <livewire:navigation-badge />
</flux:navbar.item>

livewire/navigation-badge.blade.php

<flux:badge color="sky">{{ $this->invitations }}</flux:badge>

@kyledoesdev
Copy link
Author

@kyledoesdev That's happening because your navigation item is a Livewire component containing the event listener thus it is getting re-rendered when the event is dispatched and, at that point, the route is livewire/update so the navbar.item's route does not match.

I believe if you move the navbar.item markup outside of the component, and refactor the component to only render the flux:badge, all will work as you expect, e.g.:

layout-level:

<flux:navbar.item
    href="{{ route('invites') }}"
    icon="envelope" 
    :current="request()->is('invites')"
>
    Invitations <livewire:navigation-badge />
</flux:navbar.item>

livewire/navigation-badge.blade.php

<flux:badge color="sky">{{ $this->invitations }}</flux:badge>

This somewhat patches the problem, but the badge is now not included in the accent underline of the nav item because it's a sperate badge being rendered. This works, it just doesn't have that fluid look to it:

Image

@jeffchown
Copy link

@kyledoesdev We're getting closer... I'll give this a think.

@kyledoesdev
Copy link
Author

@kyledoesdev We're getting closer... I'll give this a think.

Sounds good, I am going to continue to tinker with it as well.

@jeffchown
Copy link

@kyledoesdev Just heading out, but what about moving the word 'Invitations' into the component's render so the component is responsible for the entire menu prompt?

@kyledoesdev
Copy link
Author

@kyledoesdev Just heading out, but what about moving the word 'Invitations' into the component's render so the component is responsible for the entire menu prompt?

I think that'll do it. I can throw some tailwind on the badge to scale it better. But I can go with this one for now:

2024-12-22.12-52-21.mp4

@joshhanley
Copy link
Member

@kyledoesdev thanks for reporting! @jeffchown is correct, on update, request has changed, so the current route no longer matches. Caleb recently added wire:current to Livewire to solve this problem when using Livewire directly. But that won't work with Flux's navbars.

So even if you manage to find a work around, I think you should leave this open, so we can see if we can get Flux using something like wire:current so you don't need the work around.

@kyledoesdev
Copy link
Author

@kyledoesdev thanks for reporting! @jeffchown is correct, on update, request has changed, so the current route no longer matches. Caleb recently added wire:current to Livewire to solve this problem when using Livewire directly. But that won't work with Flux's navbars.

So even if you manage to find a work around, I think you should leave this open, so we can see if we can get Flux using something like wire:current so you don't need the work around.

Sounds good Josh! Let's leave it open for now. The work-around that @jeffchown suggested above is working fine for now for me.

@joshhanley
Copy link
Member

joshhanley commented Jan 7, 2025

I've had a look into whether we can use wire:current with Flux nav items, and at the moment the answer is no.

The reason is that Flux adds a data-current attribute to the active item and the active styling gets applied based on that. But wire:current can only be used to add classes to an element when a link is active.

So I've submitted a PR livewire/livewire#9130 to Livewire to add support for a .attr modifier for wire:current which will allow us to do wire:current.attr="data-current" to add the data-current attribute to a link when Livewire detects it is the active link.

This should allow us to update Flux at a later date to support usage of this, or a user can do it themselves by using :current="false" to disable Flux's current handling and then pass in wire:current.attr="data-current".

<flux:navbar.item href="/about" :current="false" wire:current.attr="data-current">About</flux:navbar.item>

Volt component I used for testing the Livewire PR

Volt::route('/playground/{id?}', 'playground')->name('playground');
<?php

use Livewire\Volt\Component;

new class extends Component {
    //
};
?>

<div>
    <button type="button" wire:click="$refresh">Refresh</button>
    
    <flux:navbar>
        <flux:navbar.item
            href="{{ route('playground') }}"
            :badge="5"
            badge-color="sky"
            icon="envelope"
        >
            Invitations
        </flux:navbar.item>
        <flux:navbar.item href="/playground" :current="false" wire:current.attr="data-current">Playground</flux:navbar.item>
        <flux:navbar.item href="/playground" :current="false" wire:current.attr.exact="data-current">Playground Exact</flux:navbar.item>
        <flux:navbar.item href="/playground/1" :current="false" wire:current.attr.exact="data-current">Playground 1</flux:navbar.item>
        <flux:navbar.item href="/playground/2" :current="false" wire:current.attr.exact="data-current">Playground 2</flux:navbar.item>
    </flux:navbar>
    <hr class="mt-4" />
    <a class="block" href="/playground" wire:current="font-bold">Playground</a>
    <a class="block" href="/playground" wire:current.exact="font-bold">Playground Exact</a>
    <a class="block" href="/playground/1" wire:current.exact="font-bold">Playground 1</a>
    <a class="block" href="/playground/2" wire:current.exact="font-bold">Playground 2</a>
    <flux:link class="!block" href="/playground" wire:current="font-bold">Playground</flux:link>
    <flux:link class="!block" href="/playground" wire:current.exact="font-bold">Playground Exact</flux:link>
    <flux:link class="!block" href="/playground/1" wire:current.exact="font-bold">Playground 1</flux:link>
    <flux:link class="!block" href="/playground/2" wire:current.exact="font-bold">Playground 2</flux:link>
</div>

@joshhanley
Copy link
Member

joshhanley commented Jan 7, 2025

@kyledoesdev so I've just tested what you are reporting a bit further and realised for your specific scenario you shouldn't need to set :current. Is there a reason you are doing that? If you remove it, then when the component refreshes, it works fine for me.

Like

<flux:navbar.item
            href="{{ route('playground') }}"
            :badge="5"
            badge-color="sky"
            icon="envelope"
        >

If you don't use :current then under the hood Flux knows if there has been a Livewire request or not and handles it correctly.

@kyledoesdev
Copy link
Author

@kyledoesdev so I've just tested what you are reporting a bit further and realized for your specific scenario you shouldn't need to set :current. Is there a reason you are doing that? If you remove it, then when the component refreshes, it works fine for me.

Like

<flux:navbar.item
href="{{ route('playground') }}"
:badge="5"
badge-color="sky"
icon="envelope"
>
If you don't use :current then under the hood Flux knows if there has been a Livewire request or not and handles it correctly.

I believe I was doing :current="request()->is('invites')" because without it the theme underline color of the nav item was not appearing when I was on that page's route.

<flux:navbar.item
    href="{{ route('invites') }}"
    :badge="App\Models\Invite::getPending()"
    badge-color="sky"
    icon="envelope"
>
  Invitations
</flux:navbar.item>

Image

<flux:navbar.item
    href="{{ route('invites') }}"
    :badge="App\Models\Invite::getPending()"
    badge-color="sky"
    icon="envelope"
    :current="request()->is('invites')"
>
    Invitations
</flux:navbar.item>

Image

You are correct though if I remove the :current it resolves the issue as you described above, thank you!

Thank you so much for taking the time to look into this and test it out & submitting the PR to Livewire!

@joshhanley
Copy link
Member

@kyledoesdev just to confirm it wasn't working properly before without current, but it is working now if you remove current again? If so, then great! 😁 I suspect there was an update to Flux that fixed that as originally it wasn't working properly with route matching.

No worries! 🙂

@kyledoesdev
Copy link
Author

@kyledoesdev just to confirm it wasn't working properly before without current, but it is working now if you remove current again? If so, then great! 😁 I suspect there was an update to Flux that fixed that as originally it wasn't working properly with route matching.

No worries! 🙂

That is what I think as well. I believe a flux patch that was released since me initially running into this fixed it. How did you want to proceed with this issue? Did you want to close it, or wait until livewire/livewire#9130 is merged?

@joshhanley
Copy link
Member

Ok great! I'm going to close this now, as I believe this issue is fixed. But I will leave the Livewire PR open as I think it could be a helpful feature to have 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants