Skip to content
New issue

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

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

Already on GitHub? # to your account

fix: icon position for action group in header actions #11687

Merged

Conversation

andrewdwallo
Copy link
Contributor

@andrewdwallo andrewdwallo commented Mar 5, 2024

Description

This PR attempts to fix an issue where when using iconPosition() for an ActionGroup in the getHeaderActions() method of a custom page (in my case), it was not doing anything.

I am not sure if the code I proposed here would be considered the "best" or "correct" way to do it, but I thought it would be nice for this to be able to be used.

It is worth noting that when using iconPosition() for an ActionGroup with iconButton(), placing the :icon-position prop in the view without the check I did results in the error "trim(): Argument #1 ($string) must be of type string, Filament\Support\Enums\IconPosition given" coming from the <x-filament::icon-button> component view, so maybe there is a better way to handle this.

Example

protected function getHeaderActions(): array
{
    return [
        ActionGroup::make([
            Action::make('exportCSV'),
            Action::make('exportPDF'),
        ])
            ->label('Export')
            ->button()
            ->dropdownPlacement('bottom-end')
            ->icon('heroicon-c-chevron-down')
            ->iconPosition(IconPosition::After),
    ];
}

Before

Screenshot 2024-03-04 at 10 03 28 PM

After

Screenshot 2024-03-04 at 10 03 02 PM
  • Visual changes (if any) are shown using screenshots/recordings of before and after.

Code style

  • composer cs command has been run.

Testing

  • Changes have been tested.

@andrewdwallo
Copy link
Contributor Author

Maybe a conditional check in the ActionGroup class and passing it through with extraAttributes(['icon-position' => $iconPosition]) would be better.. not sure.

@zepfietje
Copy link
Member

Shouldn't this be fixed by making a change to icon-button-group.blade.php instead?

@zepfietje zepfietje added the bug Something isn't working label Mar 5, 2024
@zepfietje zepfietje added this to the v3 milestone Mar 5, 2024
@danharrin
Copy link
Member

No, I think it needs fixing in button-group.blade.php and link-group.blade.php, similar to how we pass the icon position in button-action.blade.php and link-action.blade.php

@danharrin
Copy link
Member

Found the problem and fixed with an alt solution

@danharrin danharrin merged commit b7048da into filamentphp:3.x Mar 5, 2024
10 checks passed
@andrewdwallo
Copy link
Contributor Author

Sounds good, thanks!

@andrewdwallo andrewdwallo deleted the fix/action-group-icon-position branch March 5, 2024 22:09
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants