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.

Sign up for GitHub

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

Jump to bottom

Add <UpdateButton> to let users update the current record #9088

Merged
merged 10 commits into from
Jul 10, 2023
Merged

Add <UpdateButton> to let users update the current record #9088

merged 10 commits into from
Jul 10, 2023

Conversation

djhi
Copy link
Collaborator

@djhi djhi commented Jul 10, 2023 •
edited
Loading

updatebutton.webm

@djhi djhi added RFR Ready For Review WIP Work In Progress and removed RFR Ready For Review labels Jul 10, 2023
@djhi djhi added RFR Ready For Review and removed WIP Work In Progress labels Jul 10, 2023
slax57
slax57 requested changes Jul 10, 2023
View reviewed changes
docs/UpdateButton.md Outdated Show resolved Hide resolved
docs/UpdateButton.md Outdated Show resolved Hide resolved
docs/UpdateButton.md Outdated Show resolved Hide resolved
docs/navigation.html Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/button/UpdateButton.tsx Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/button/UpdateButton.tsx Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/button/UpdateButton.tsx Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/button/UpdateWithConfirmButton.spec.tsx Outdated Show resolved Hide resolved
docs/UpdateButton.md Show resolved Hide resolved
@djhi @slax57
Apply suggestions from code review
6ef96f1 
Co-authored-by: Jean-Baptiste Kaiser <[email protected]>
slax57
slax57 requested changes Jul 10, 2023
View reviewed changes
docs/UpdateButton.md Outdated Show resolved Hide resolved
slax57
slax57 requested changes Jul 10, 2023
View reviewed changes
Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

2 more issues unfortunately:

  • Story 'Side effects' raises an error since we removed the onSuccess and onError props
  • In undoable mode, I believe refresh() is called before the mutation has actually happened, which causes the old value to come back for the duration of the notification. IMO in the default onSuccess we should not call refresh()

@slax57 slax57 added this to the 4.13.0 milestone Jul 10, 2023
@slax57 slax57 merged commit 171593e into next Jul 10, 2023
@slax57 slax57 deleted the update-button branch July 10, 2023 15:56
@fzaninotto fzaninotto changed the title Add UpdateButton Add <UpdateButton> to let users update the current record Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slax57 slax57 slax57 approved these changes

Assignees
No one assigned
Labels
RFR Ready For Review
Projects
None yet
Milestone
4.13.0
Development

Successfully merging this pull request may close these issues.
2 participants
@djhi @slax57