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

[added] Introduce onAfterClose callback prop #724

Merged
merged 2 commits into from
Dec 17, 2018
Merged

[added] Introduce onAfterClose callback prop #724

merged 2 commits into from
Dec 17, 2018

Conversation

fernandofleury
Copy link
Contributor

@fernandofleury fernandofleury commented Dec 14, 2018

Fixes #708 .

Since the onRequestClose won't be executed when isOpen changes outside of the controller scope, I think it's reasonable to give a single handler to general closing.

This would allow for some more centralised solutions like:

const modalProps = {
  onAfterOpen: disableBodyScroll,
  onAfterClose: enableBodyScroll,
}

whereas today you have to do:

const modalProps = {
  onAfterOpen: disableBodyScroll,
  onRequestClose:  enableBodyScroll
}

closeModal () {
  this.setState({ isOpen: false })
  enableBodyScroll();
}

Changes proposed:

  • Introduce onAfterClose callback prop

Upgrade Path (for changed or removed APIs):

Acceptance Checklist:

  • The commit message follows the guidelines in CONTRIBUTING.md.
  • Documentation (README.md) and examples have been updated as needed.
  • If this is a code change, a spec testing the functionality has been added.
  • If the commit message has [changed] or [removed], there is an upgrade path above.

@coveralls
Copy link

coveralls commented Dec 14, 2018

Coverage Status

Coverage increased (+0.06%) to 86.621% when pulling 9eb12e0 on fernandofleury:master into d4a8a32 on reactjs:master.

@diasbruno
Copy link
Collaborator

Hi @fernandofleury. Thanks for implementing this. Good job!

@fernandofleury
Copy link
Contributor Author

@diasbruno I've updated as you requested

@diasbruno diasbruno merged commit 988f55a into reactjs:master Dec 17, 2018
@diasbruno
Copy link
Collaborator

Released version 3.8.1.

@chris-pearce
Copy link

Is there any reason why this isn't anywhere in the docs?

http://reactcommunity.org/react-modal/?q=#installation

programmer4web pushed a commit to programmer4web/react-modal that referenced this pull request Oct 5, 2020
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.

onAfterClose prop passing
4 participants