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

Add ActionGroup Support To Header/Footer Actions On Infolists #14836

Open
wants to merge 2 commits into
base: 3.x
Choose a base branch
from

Conversation

Magnesium38
Copy link
Contributor

Description

I was surprised when I tried to use an ActionGroup in my code last week to discover an except about a missing getName() method. I searched through the existing issues and found a couple of other people who had the same issue (1, 2, 3). I had some time over the weekend to dig into it and get everything hooked up.

I had noticed #13172 and used that as a jumping off point to get familiar with the relevant code. My first attempt was a more direct attempt to solve the missing name problem, but an ActionGroup needing a name didn't feel great to work with. Instead, I reconsidered why it needed a name in the first place. The names are used so that an action can be retrieved later with getAction(). Since the group shouldn't have a reason to be retrieved I put them onto the array without a name by using the numeric index instead. This does create a mixed array, but all of the code in this package handles it okay already. Additionally, the docblock of array<Action | ActionGroup> omits the key parameter which implies array<int | string, Action | ActionGroup> so there isn't any breakage here.

I did write a basic test to prove that grouped header and footer actions can be invoked and installed this fork into a local project and tested it manually as well and had no issues.

I know that there's also requests for this feature on the forms side (#13234) and I'm open to doing a PR for that side as well, but I needed the infolist side so I started here. If this gets accepted I'll likely quick knock out the same changes on the forms side. That PR should then be able to close #14155.

One thing that I have not done is make any documentation changes yet. From what I've seen, it appears that most people try group actions after finding the Actions Grouping Actions documentation. I would be open to writing up the documentation using the table's grouping actions documentation as a starting point, but I didn't want to spend all the time capturing screenshots and all that if this wasn't going to be accepted.

Visual changes

This is the block that I was using to test whether things were working when used on a real project.

ActionGroup::make([
	Action::make('first')
		->action(fn (array $data) => dd('this', $data))
		->form([
			TextInput::make('text'),
		]),
]),
Action::make('test')
	->action(fn (array $data) => dd('this', $data))
	->form([
		TextInput::make('text'),
	]),
ActionGroup::make([
	Action::make('second')
		->action(fn (array $data) => dd('this', $data))
		->form([
			TextInput::make('text'),
		]),
]),

image
image

They resulted in header actions that were grouped and sorted as expected.

Functional changes

  • Code style has been fixed by running the composer cs command.
  • Changes have been tested to not break existing functionality.
  • Documentation is up-to-date.

@zepfietje zepfietje added this to the v3 milestone Nov 20, 2024
@zepfietje zepfietje added the enhancement New feature or request label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

Support action groups in section header and footer actions
2 participants