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 support for Cluster #458

Merged
merged 6 commits into from
May 16, 2024
Merged

Add support for Cluster #458

merged 6 commits into from
May 16, 2024

Conversation

atmonshi
Copy link
Contributor

@atmonshi atmonshi commented Mar 12, 2024

still draft... ... one moment plz.

the issue

@atmonshi
Copy link
Contributor Author

so instead of using the plugin configuration, as a work around we can use the config only, and its work as expected but its not customizable per panel.
what do you think of this approach!

I left the code in the plugin class as comments to get back to it later.

@atmonshi atmonshi marked this pull request as ready for review March 31, 2024 17:40
@awcodes
Copy link
Owner

awcodes commented Apr 22, 2024

Do you happen to have a demo repo I can try all this out on? Honestly, I haven't even used the cluster feature so I don't have anything setup to test against.

@atmonshi
Copy link
Contributor Author

ya I was procrastinating creating a reproduce repo to report to filament 😅

but here it is:
https://github.com/atmonshi/issue-cluster

and I am using this fork:
https://github.com/atmonshi/filament-curator/tree/cluster-panel

you'll see the error "Call to a member function getPlugin() on null", so you can disable ->cluster(Life::class) here:

https://github.com/atmonshi/issue-cluster/blob/05e27590c65b5f60ae53bf07728d918f3a5adffc/app/Providers/Filament/AdminPanelProvider.php#L47

@awcodes
Copy link
Owner

awcodes commented Apr 22, 2024

Awesome. Thanks. Just didn't want you to think I was ignoring this or didn't care. 😅

@awcodes
Copy link
Owner

awcodes commented Apr 27, 2024

Is this not ready or a bug in filament? I couldn't get it to work.

@atmonshi
Copy link
Contributor Author

this PR is using the config file only, so its like half way there, a temp solution.

reported the bug here:
filamentphp/filament#12532

if this fixed then I'll update this PR to use the plugin class based configuration, so it will be customizable per panel.

@awcodes
Copy link
Owner

awcodes commented Apr 28, 2024

Ok. Can you change it back to draft until it's ready?

@atmonshi atmonshi marked this pull request as draft April 28, 2024 18:50
@atmonshi
Copy link
Contributor Author

hey @awcodes, so it seems (cluster configuration from the panel) is not possible, as Dan explain:

as the cluster needs to be read when the routing is registered, and the panel may not be booted at that point so the plugins aren't there.

this PR is ready for a cluster configuration from the panel config file.

so if you want to merge it or not totally fine :) 🍻

@atmonshi atmonshi marked this pull request as ready for review May 14, 2024 20:44
@awcodes
Copy link
Owner

awcodes commented May 14, 2024

So how does this work? Lol.

@atmonshi
Copy link
Contributor Author

atmonshi commented May 14, 2024

😂
will simply in config/curator.php:

'cluster' => \App\Filament\Clusters\Cms::class,

what I was hopping was to customize it per panel:

CuratorPlugin::make()
       ->cluster(\App\Filament\Clusters\Cms::class)

but got greedy, will have to convince the client its "better" this way lol

@awcodes
Copy link
Owner

awcodes commented May 16, 2024

So, how is this different than over ridding the resource which is already possible?

@atmonshi
Copy link
Contributor Author

less files to copy to the app :)
same idea for navigation_label and other resources configuration.

@awcodes awcodes merged commit 57c8b9c into awcodes:3.x May 16, 2024
1 check passed
@atmonshi atmonshi deleted the cluster branch May 17, 2024 04:07
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 this pull request may close these issues.

2 participants