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

[any-plat][NavView] deselecting item by selecting another leave the former in "Selected" state #107

Closed
5 of 9 tasks
Xiaoy312 opened this issue Feb 4, 2021 · 5 comments
Closed
5 of 9 tasks
Assignees
Labels
area/canary kind/bug Something isn't working
Milestone

Comments

@Xiaoy312
Copy link
Contributor

Xiaoy312 commented Feb 4, 2021

Current behavior

Deselecting item by selecting another leave the former in "Selected" state:
20210204 mux-navview deselect

Expected behavior

The deselected item should go back to "Normal" or "Checked" state

How to reproduce it (as minimally and precisely as possible)

Checkout master
Comment out App.AddNavigationItems.NavViewItemVisualStateFix method
Launch the app on any platform
Click on different navigation items

Environment

Nuget Package: Uno.UI
Package Version(s): 3.5.0-dev.256
Affected platform(s):

  • iOS
  • Android
  • WebAssembly
  • WebAssembly renders for Xamarin.Forms
  • Windows
  • Build tasks

Visual Studio:

  • 2017 (version: )
  • 2019 (version: )
  • for Mac (version: )

Relevant plugins: n/a

Anything else we need to know?

Current workaround is to force reset the visual state when IsSelected becomes false.

@Xiaoy312 Xiaoy312 added kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification. labels Feb 4, 2021
@jeromelaban jeromelaban added area/canary and removed triage/untriaged Indicates an issue requires triaging or verification. labels Feb 5, 2021
@jeromelaban jeromelaban added this to the 3.5 milestone Feb 5, 2021
@MartinZikmund
Copy link
Member

@jeromelaban So far I am unable to figure this one out, especially as it even happens on UWP too. Weirdly enough, using similar styles in a blank app I cannot reproduce it. Luckily the workaround works, so it is not a blocker.

@francoistanguay francoistanguay modified the milestones: 3.5, 3.6 Feb 8, 2021
@Xiaoy312
Copy link
Contributor Author

Xiaoy312 commented Feb 17, 2021

The reason why the default style works, is that there is no distinction between selected vs normal state other than the "SelectionIndicator" that is managed by the NavView (the parent, not NVItem).
---
It seems that microsoft has fixed this issue in 2.5.0, but it WAS present in 2.4.3:

20210217 nvi selected 2.4.3.gif

20210217 nvi selected 2 4 3
^ notice that the previous item state never resets back to "Normal"

20210217 nvi selected 2.5.0.gif

20210217 nvi selected 2 5 0

Here is the PR containing the fix: microsoft/microsoft-ui-xaml#2625
And, the sample used for the gifs: NavViewDeselection243.zip

I would assume we need to port that into uno as well.

@Xiaoy312
Copy link
Contributor Author

@MartinZikmund ^

@MartinZikmund
Copy link
Member

@Xiaoy312 that is curious as I ported NavView based on the current version around December 2020, so these changes should already be included. Can you please compare the relevant changes with our code to see if I didn't miss something somewhere? Or maybe some part of the style is different?

@Xiaoy312 Xiaoy312 self-assigned this Mar 17, 2021
@Xiaoy312
Copy link
Contributor Author

note
issue can no longer reproduced on current master even after commenting out the body of `NavViewItemVisualStateFix`
    - ios: OK iPad Pro (9.7in) iOS 14.4
    - wasm: OK
    - win: OK

checking out a version when the issue is posted
    f0803a77 2021-02-04 Merge pull request #101 from unoplatform/dev/xygu/20210128/muxc-nav-view
    ^ after commenting out the body of `NavViewItemVisualStateFix`
        > still cant repro on uno..?
        - win: FAIL
        - ios: OK iPad Pro (9.7in) iOS 14.4
        - wasm: OK
        - android: OK Tablet M-DPI 10.1in Q 10.0
    ^ the visual state on `MUX_NavigationViewItemPresenterStyleWhenOnLeftPane` appears to be okay for uno
    ^ package version 3.5.0-dev.256 matches the one in the issue

same issue cant be reproduced with the attached sample either on android or wasm; it is only on uwp that it fails
    > previous item resets back to `Normal` state from `Selected` (this is the expected behavior)
    ^ note: the `Loaded` handler needs to be moved into a Button.Click, as Loaded seems to be too early, on non-uwp platforms

Cant seem to reproduce the issue anymore, despite going back to the older commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/canary kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants