This repository was archived by the owner on Jun 6, 2025. It is now read-only.
Use setState callback to ensure state despite asynchrony #406
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tadjohnston
suggested changes
Sep 27, 2018
Collaborator
tadjohnston
left a comment
tadjohnston
left a comment
There was a problem hiding this comment.
Please use git cz to commit. It allows us to autogenerate changelogs and verion bump based on what you inputed using commitizen.
Collaborator
|
Other than the one comment, 👍 |
jamessral
force-pushed
the
jsral/fix-set-state-callback
branch
from
b14d58f to
87dcf73
Compare
gabepages
previously approved these changes
Sep 28, 2018
MiLandry
previously approved these changes
Sep 28, 2018
jamessral
force-pushed
the
jsral/fix-set-state-callback
branch
from
87dcf73 to
882b6b3
Compare
Contributor
Author
|
@gabepages @winstonmain Sorry, I didn't see the reviews from y'all. I'm still checking this out by linking it to rent-js and my premature rebase got rid of your comments |
jamessral
force-pushed
the
jsral/fix-set-state-callback
branch
from
882b6b3 to
5670e67
Compare
jamessral
commented
Sep 28, 2018
| } | ||
| } | ||
|
|
||
| @autobind |
jamessral
force-pushed
the
jsral/fix-set-state-callback
branch
from
5670e67 to
a475f41
Compare
… for handleDocumentClick affects: @rentpath/react-ui-core It is possible with event queueing that the caught event is that of the immediate child. Include a new and specific data attribute to reliably check to see if the Dropdown or the anchor is the target of the click. Also clean up setState logic in toggleVisibility to not rely on state before and after setState call.
jamessral
force-pushed
the
jsral/fix-set-state-callback
branch
from
a475f41 to
84e5ee9
Compare
davidthrasher
approved these changes
Oct 1, 2018
dgavigan
approved these changes
Oct 1, 2018
tadjohnston
approved these changes
Oct 1, 2018
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depending on
this.statebefore and after the call can cause a subtle bug (in this case calling the onVisibilityChange callback twice when a parent component callssetState). Using the callback syntax to set state and the after set callback ensures that everything happens synchronously and the values inthis.stateare what we expect.Edit: This is the real issue below
It is possible with event queueing that the caught event is that of the immediate child.
Include a
new and specific data attribute to reliably check to see if the Dropdown or the anchor is the target
of the click.
Also clean up setState logic in toggleVisibility to not rely on state before and
after setState call.