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

Admin Generator (Future): Allow Grid independent of Stack: Inject add and edit button from outside #2171

Merged
merged 7 commits into from
Jul 15, 2024

Conversation

Ben-Ho
Copy link
Contributor

@Ben-Ho Ben-Ho commented Jun 13, 2024

  • Add new config-options toolbarActionProp and rowActionProp, because this shouldn't be the most common case
  • Allows providing a complete button, besides changing the default stack-page behavior this also enables renaming the button which is often linked.
  • If toolbarActionProp or rowActionProp is enabled but the prop is not set no button is rendered. This allows using the same grid like a grid with { ..., add: false, ...} and add: true configuration at the same time.

@Ben-Ho Ben-Ho requested review from nsams and johnnyomair June 13, 2024 09:38
@Ben-Ho Ben-Ho self-assigned this Jun 13, 2024
@Ben-Ho Ben-Ho changed the title Admin Generator (Future): Allow set add-button via props Admin Generator (Future): Remove stack-specific button from generated grid Jun 13, 2024
@Ben-Ho Ben-Ho changed the title Admin Generator (Future): Remove stack-specific button from generated grid Admin Generator (Future): Remove stack-specific buttons from generated grid Jun 13, 2024
@Ben-Ho Ben-Ho force-pushed the admin-gen-configurable_forward-add-button branch from bfb13fe to 2a6080a Compare June 13, 2024 14:22
@nsams nsams changed the title Admin Generator (Future): Remove stack-specific buttons from generated grid Grid independent of Stack: Inject stack-specific buttons from page into grid Jun 14, 2024
@Ben-Ho Ben-Ho force-pushed the admin-gen-configurable_forward-add-button branch from c28e011 to ee26209 Compare June 27, 2024 09:54
@Ben-Ho Ben-Ho changed the title Grid independent of Stack: Inject stack-specific buttons from page into grid Admin Generator (Future): Allow Grid independent of Stack: Inject add and edit button from outside Jun 27, 2024
Copy link
Collaborator

@johnnyomair johnnyomair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about using the name actions instead of buttons?

actionsProps: true

toolbarActions or primaryAction

rowAction

Would be more versatile

@Ben-Ho
Copy link
Contributor Author

Ben-Ho commented Jun 27, 2024

What do you think about using the name actions instead of buttons?

this could probably lead to confusion with #1913. But one of the two buttons is actually an action-button... what do you think about navigationButtonProps?

Or your example primaryActionProp | toolbarActionProp and rowActionProp but allow setting them separately. This could also go well with #1913, defining some custom actions and setting primaryActionProp=true...

@johnnyomair
Copy link
Collaborator

what do you think about navigationButtonProps?

This would imply that it must always be a button and a navigation, while it doesn't have to be either IMO.

Or your example primaryActionProp | toolbarActionProp and rowActionProp but allow setting them separately.

Would be okay for me.

@nsams what do you think?

@Ben-Ho
Copy link
Contributor Author

Ben-Ho commented Jul 9, 2024

Or your example primaryActionProp | toolbarActionProp and rowActionProp but allow setting them separately.

Would be okay for me.

then I'll go with toolbarActionProp and rowActionProp because it contains the location. and primary is defined by styling so this isn't correct either.

@johnnyomair johnnyomair merged commit f350db6 into next Jul 15, 2024
11 checks passed
@johnnyomair johnnyomair deleted the admin-gen-configurable_forward-add-button branch July 15, 2024 14:01
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.

3 participants