Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Doc] Improve the side effect hooks documentation #6388

Merged
merged 2 commits into from
Jun 28, 2021
Merged

Conversation

fzaninotto
Copy link
Member

image

@fzaninotto fzaninotto added the RFR Ready For Review label Jun 23, 2021
Copy link
Collaborator

@djhi djhi left a comment

Choose a reason for hiding this comment

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

👍

const notify = useNotify();

const onSuccess = () => {
notify(`Changes saved`, undefined, undefined, true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should probably ask for a single option object instead of 5 args imo. Maybe we can support both until we deprecate the 5 args signature?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice idea, but let's do it in another PR

@djhi djhi added this to the 3.16.4 milestone Jun 28, 2021
@djhi djhi merged commit 9488f8e into master Jun 28, 2021
@djhi djhi deleted the doc-side-effect-hooks branch June 28, 2021 08:41
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants