-
Notifications
You must be signed in to change notification settings - Fork 698
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
NavigationViewItem input handling fixes #2625
Merged
Merged
Conversation
This file contains 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
msft-github-bot
added
the
needs-triage
Issue needs to be triaged by the area owners
label
Jun 8, 2020
RBrid
commented
Jun 8, 2020
RBrid
commented
Jun 8, 2020
StephenLPeters
added
team-Controls
Issue for the Controls team
and removed
needs-triage
Issue needs to be triaged by the area owners
labels
Jun 8, 2020
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
ojhad
approved these changes
Jun 9, 2020
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
ojhad
reviewed
Jun 10, 2020
Kinnara
added a commit
to Kinnara/ModernWpf
that referenced
this pull request
Jun 12, 2020
ranjeshj
added
the
needs-cherrypicktorelease
PR tagged for cherry-pick to the current release branch (but not yet picked)
label
Jun 16, 2020
ranjeshj
removed
the
needs-cherrypicktorelease
PR tagged for cherry-pick to the current release branch (but not yet picked)
label
Aug 12, 2020
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
These changes address a series of bugs in the NavigationViewItem implementation:
#1 NavigationViewItem does not update its visual state when its IsSelected property changed.
Fix: Listening to IsSelected change notifications to refresh visual state.
#2 PonterMoved does not update NavigationViewItem visual state in WinUI3.
Fix: Listening to PointerMoved to update visual state when pointer is not thought to be over.
#3 Mouse pointer is not captured in PointerPressed.
Fix: Pointer is captured in PointerPressed and released in PointerReleased, or when pointer is lost.
#4 IsEnabled change does not update NavigationViewItem visual state.
Fix: Listening to NavigationViewItem.IsEnabledChanged to clear the state flags and refresh visual state.
#5 Beginning of OnApplyTemplate does not reset template parts, or clear event handlers.
Fix: All control parts and handler are cleared at beginning of OnApplyTemplate to avoid leftover incorrect fields.
#6 Pointer input handlers that reset internal flags are not invoked when args.Handled is true causing bugs.
Fix: Input handlers that reset flags are not skipped thanks to the handledEventsToo=True feature.
#7 Multi finger inputs are not handled properly. Removing second finger clears pressed state for instance.
Fix: Only one pointer Id is tracked for a given NavigationViewItem now. Any newcomer is ignored.
o Also did some dead code cleanup, and code cleanup in general.
o Did some ad-hoc testing, expanded controls test app and NavigationView test.
(These changes also address 26317023 once pushed into WinUI)