Fix issue in modal Selection handling#3836
Merged
Merged
Conversation
brwatt
approved these changes
Jan 31, 2018
ThomasMichon
force-pushed
the
selection-modal
branch
from
c780d0e to
a1cfd18
Compare
joschect
approved these changes
Jan 31, 2018
| public setModal(isModal: boolean): void { | ||
| if (this._isModal !== isModal) { | ||
| this.setChangeEvents(true); | ||
| this.setChangeEvents(false); |
Contributor
There was a problem hiding this comment.
Would you mind adding a comment as to why this is necessary?
Member
Author
There was a problem hiding this comment.
Do you want the comment in all the other places?
Contributor
There was a problem hiding this comment.
I should have looked more closely. For some reason I thought this was the only place the change should occur. Ignore this!
| private _forceListUpdates() { | ||
| this._pendingForceUpdate = false; | ||
|
|
||
| if (this._groupedList) { |
Contributor
There was a problem hiding this comment.
Part of me feels like we should make forceUpdate a property that gets passed down to the grouped/list so that it maintains the lifecycle better. Probably doesn't need to be done now.
Member
Author
There was a problem hiding this comment.
We should get rid of forceUpdate.
gokunymbus
added a commit
that referenced
this pull request
Feb 1, 2018
This reverts commit c304b01.
gokunymbus
added a commit
that referenced
this pull request
Feb 1, 2018
* Revert "Updating config files in @uifabric/icons package (#3851)" This reverts commit 952ad79. * Revert "Addressing Issue #3776 - Sticky component loops forever if background-color is not set for page (#3833)" This reverts commit ddc5440. * Revert "Fix issue in modal Selection handling (#3836)" This reverts commit c304b01. * Revert "Adding new WOFF files to the @uifabric/icons/fonts folder (#3848)" This reverts commit 66a91aa. * Revert "Controlled TextFields (#3796)" This reverts commit b17a45f.
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
Overview
This change fixes in issue where use of
_setChangeEventsinSelectionwas incorrectly flipped when handling the modal selection state. This causes change notifications to fail in downstream UX.This change also fixed an issue where touch events were not properly triggering the modal selection state in some edge cases.