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

rename lifecycle methods like props #2402

Closed
giladgray opened this issue Apr 18, 2018 · 4 comments
Closed

rename lifecycle methods like props #2402

giladgray opened this issue Apr 18, 2018 · 4 comments
Assignees
Milestone

Comments

@giladgray
Copy link
Contributor

giladgray commented Apr 18, 2018

popoverDidOpen/popoverWillClose etc props are inconsistent with other callback prop names. I propose renaming these to:

  • popoverWillOpen => onOpening
  • popoverDidOpen => onOpened
  • popoverWillClose => onClosing
  • popoverDidClose => onClosed

The same changes will happen to the corresponding Overlay props. Note that the existing onClose callback fits nicely in this flow. And I'd happily add an onOpen for consistency.

This is identical to CSSTransitionGroup's lifecycle prop names.

@giladgray giladgray added this to the 3.0.0 milestone Apr 18, 2018
@llorca
Copy link
Contributor

llorca commented Apr 18, 2018

I thought it was pretty elegant to mirror the React component lifecycle naming pattern, I wonder why CSSTG didn't go that route

@llorca
Copy link
Contributor

llorca commented May 3, 2018

answer from Gilad: because they're not methods but props

then, what about didOpen willOpen didClose willClose? or onDidOpen onWillOpen onDidClose onWillClose?

@llorca
Copy link
Contributor

llorca commented May 21, 2018

do the original proposal

@giladgray
Copy link
Contributor Author

note that #2335 is also about a popover lifecycle bug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants