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

[Possible Bug]Update QuickCreateMenu.php #42

Closed
wants to merge 1 commit into from
Closed

Conversation

brkfun
Copy link

@brkfun brkfun commented Feb 14, 2024

Create action needs a header.
Cause of null set over, it causes "model name"(ie. user instead of User) displays over modal and slideovers.

create action needs a header because of null set over it causes model name displayed over modal and slideovers.
@awcodes
Copy link
Owner

awcodes commented Feb 14, 2024

If this is a bug it is in Filament Core. The CreateAction is already calling ->getModelLabel() internally to set the modal heading.

https://github.com/filamentphp/filament/blob/4d2528c67ead35c30df839b6e0c896722ef8a90f/packages/actions/src/CreateAction.php#L33

@awcodes awcodes closed this Feb 14, 2024
@brkfun
Copy link
Author

brkfun commented Feb 15, 2024

it cannot call for resources they are build from different class so thats why i added that line. You can also check out or i'll show an example if thats what you need.

@awcodes
Copy link
Owner

awcodes commented Feb 15, 2024

I tried your pr and it had the same outcome. User was lowercase and not uppercase.

@brkfun
Copy link
Author

brkfun commented Feb 15, 2024

you need to add $modelLabel to your resource to change the title also. If you forgot that.

Let me show you an example :

class SimpleResource extends Resource
{
    public static ?string $modelLabel = 'Your Header';

    public static function form(Form $form): Form
    {
        // Your form goes here.
    }

    public static function getPages(): array
    {
        return [
            'index' => ListPage::route('/'), // for make your resource simple if you add create or edit it directly goes to that route so we can open modal or slideover without give any more information to here.
        ];
    }
}

after simple resource has been done you can directly call from your plugin with include.

    ->plugin(
        QuickCreatePlugin::make()->includes([
            SimpleResource::class,
        ])
    )

and it will show the title as you change modelLabel

In "Model based resources" you can change your title as same way as this line i create.

@awcodes
Copy link
Owner

awcodes commented Feb 15, 2024

That still doesn't make sense, the resource is still calling the same getModelLabel() in both cases. Internally on Filament's side it's still reading from the resource in the create action.

@brkfun
Copy link
Author

brkfun commented Feb 15, 2024

Yeah you are right. it doesn't make sense but it works because of action is no depended or extended by any type of page or any type of resource. But at the end it works like this and i can change the title.
in example create action goes for $this->$modelLabel but in your package you are creating an action without calling modelLabel from resource.

@awcodes
Copy link
Owner

awcodes commented Feb 15, 2024

What version of Filament are you on?

https://github.com/filamentphp/filament/blob/4d2528c67ead35c30df839b6e0c896722ef8a90f/packages/actions/src/CreateAction.php#L31 <- Filament uses $this->getModelLabel() in the CreateAction class.

@brkfun
Copy link
Author

brkfun commented Feb 15, 2024

i'm** using 3.2.30 which is latest version i have the same line but, as i said you are creating the action from scratch it doesn't know the resources modelLabel because you are not passing it to that.
CreateAction created by your package over menu it needs modelLabel you have to pass it from our resources to your created CreateAction.

@awcodes
Copy link
Owner

awcodes commented Feb 15, 2024

I'll look into it some more when I get a chance. But it still doesn't sound right to have to explicitly set a label on the resource property for this to work.

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