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.

Sign up for GitHub

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

Jump to bottom

ItemsRepeater OnElementPrepared is called after NavigationViewItem.OnApplyTemplate #4689

Open
9 of 24 tasks
MartinZikmund opened this issue Dec 8, 2020 · 1 comment
Open
9 of 24 tasks

ItemsRepeater OnElementPrepared is called after NavigationViewItem.OnApplyTemplate #4689

MartinZikmund opened this issue Dec 8, 2020 · 1 comment
Labels
difficulty/challenging 🤯 Categorizes an issue for which the difficulty level is reachable with internals understanding kind/bug Something isn't working project/layout 🧱 Categorizes an issue or PR as relevant to layouting and containers (Measure/Arrange, Collections,..) project/navigation-lifecycle 🧬 Categorizes an issue or PR as relevant to the navigation and lifecycle (NavigationView, AppBar, ...) project/third-party 3️⃣ Categorizes an issue or PR as relevant to 3rd party libraries

Comments

@MartinZikmund
Copy link
Member

Current behavior

In UWP, the NavigationVIewItem.SetNavigationViewParent method is called before OnApplyTemplate, which is needed to properly initialize the items and NavigationVIewItemPresenters. The SetNavigationViewParent is called from OnRepeaterElementPrepared handler, which unfortunately runs after item's OnApplyTemplate executes in Uno (reproduced on Android, WASM, Skia, probably happens on others too). In addition, I noticed that when I run the HierarchicalNavigationViewMarkup test page (in NavigationView section of samples app), an assert fails in VirtualizationInfo.MoveOwnershipToLayoutFromElementFactory, maybe it is related somehow. Currently I have worked around the SetNavigationViewParent problem by walking up the Visual tree (see the NavigationViewItemBase.FindAncestorNavigationView method), but that does help on Skia and WASM only, whereas on Android and iOS, the parent is not yet set when OnApplyTemplate is called.

The problem is likely related to the lifetime events order.

Expected behavior

OnElementPrepared should be called before OnApplyTemplate.

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

See above.

Workaround

Currently worked around in ItemsRepeater and NavigationView source.

Environment

Nuget Package:

  • Uno.UI / Uno.UI.WebAssembly / Uno.UI.Skia
  • Uno.WinUI / Uno.WinUI.WebAssembly / Uno.WinUI.Skia
  • Uno.SourceGenerationTasks
  • Uno.UI.RemoteControl / Uno.WinUI.RemoteControl
  • Other:

Nuget Package Version(s):

Affected platform(s):

  • iOS
  • Android
  • WebAssembly
  • WebAssembly renderers for Xamarin.Forms
  • macOS
  • Skia
    • WPF
    • GTK (Linux)
    • Tizen
  • Windows
  • Build tasks
  • Solution Templates

IDE:

  • Visual Studio 2017 (version: )
  • Visual Studio 2019 (version: )
  • Visual Studio for Mac (version: )
  • Rider Windows (version: )
  • Rider macOS (version: )
  • Visual Studio Code (version: )

Relevant plugins:

  • Resharper (version: )

Anything else we need to know?
@MartinZikmund MartinZikmund added kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification project/third-party 3️⃣ Categorizes an issue or PR as relevant to 3rd party libraries and removed triage/untriaged Indicates an issue requires triaging or verification labels Dec 8, 2020
MartinZikmund added a commit to MartinZikmund/Uno that referenced this issue Dec 8, 2020
@MartinZikmund
chore(NavigationView): NavigationViewItem workarounds
9959b55 
- Uno specific workaround for initialization problems - issue unoplatform#4689
- Temporary uncomment of assert in ItemsRepeater VirtualizationInfo - issue unoplatform#4691
@jeromelaban jeromelaban mentioned this issue Dec 9, 2020
Open
9 tasks
MartinZikmund added a commit to MartinZikmund/Uno that referenced this issue Dec 10, 2020
@MartinZikmund
chore(NavigationView): NavigationViewItem workarounds
c0be718 
- Uno specific workaround for initialization problems - issue unoplatform#4689
- Temporary uncomment of assert in ItemsRepeater VirtualizationInfo - issue unoplatform#4691
MartinZikmund added a commit to MartinZikmund/Uno that referenced this issue Dec 10, 2020
@MartinZikmund
chore(NavigationView): NavigationViewItem workarounds
7817724 
- Uno specific workaround for initialization problems - issue unoplatform#4689
- Temporary uncomment of assert in ItemsRepeater VirtualizationInfo - issue unoplatform#4691
MartinZikmund added a commit to MartinZikmund/Uno that referenced this issue Dec 16, 2020
@MartinZikmund
chore(NavigationView): NavigationViewItem workarounds
eb36a5f 
- Uno specific workaround for initialization problems - issue unoplatform#4689
- Temporary uncomment of assert in ItemsRepeater VirtualizationInfo - issue unoplatform#4691
ajpinedam pushed a commit to ajpinedam/uno that referenced this issue Dec 17, 2020
@MartinZikmund @ajpinedam
chore(NavigationView): NavigationViewItem workarounds
0ac1c42 
- Uno specific workaround for initialization problems - issue unoplatform#4689
- Temporary uncomment of assert in ItemsRepeater VirtualizationInfo - issue unoplatform#4691
MartinZikmund added a commit to MartinZikmund/Uno that referenced this issue Dec 19, 2020
@MartinZikmund
chore(NavigationView): NavigationViewItem workarounds
a2589ce 
- Uno specific workaround for initialization problems - issue unoplatform#4689
- Temporary uncomment of assert in ItemsRepeater VirtualizationInfo - issue unoplatform#4691
MartinZikmund added a commit to MartinZikmund/Uno that referenced this issue Dec 20, 2020
@MartinZikmund
chore(NavigationView): NavigationViewItem workarounds
065e5ec 
- Uno specific workaround for initialization problems - issue unoplatform#4689
- Temporary uncomment of assert in ItemsRepeater VirtualizationInfo - issue unoplatform#4691
MartinZikmund added a commit to MartinZikmund/Uno that referenced this issue Dec 20, 2020
@MartinZikmund
chore(NavigationView): NavigationViewItem workarounds
5d1bf47 
- Workarounds to make NavigationViewItems initialize properly
- Now builds on Android
- NavigationViewItem is properly initialized on .NET Standard targets
- Uno specific workaround for initialization problems - issue unoplatform#4689
- Temporary uncomment of assert in ItemsRepeater VirtualizationInfo - issue unoplatform#4691
- Move pane toggle buttons to the bottom of control template to work around Canvas.ZIndex
- Fix UWP build
MartinZikmund added a commit to MartinZikmund/Uno that referenced this issue Dec 20, 2020
@MartinZikmund
chore(NavigationView): NavigationViewItem workarounds
9e5bb52 
- Workarounds to make NavigationViewItems initialize properly
- Now builds on Android
- NavigationViewItem is properly initialized on .NET Standard targets
- Uno specific workaround for initialization problems - issue unoplatform#4689
- Temporary uncomment of assert in ItemsRepeater VirtualizationInfo - issue unoplatform#4691
- Move pane toggle buttons to the bottom of control template to work around Canvas.ZIndex
- Fix UWP build
@MartinZikmund
Copy link
Member Author

The OnApplyTemplate order has effects on NavigationViewPresenter as well (which is nested in the NavigationViewItem)

MartinZikmund added a commit to MartinZikmund/Uno that referenced this issue Dec 30, 2020
@MartinZikmund
fix: Make NavigationView Settings item selectable
69318b4 
- Fixes unoplatform#4809 - in some cases on some Uno targets, the lifecycle events are in such order that causes presenters of dynamically added NavigationViewItems not to be initialized properly. These changes work around this problem when selection indicator is requeste. These changes can be reverted when unoplatform#4689 is fixed.
@jeromelaban jeromelaban added the project/layout 🧱 Categorizes an issue or PR as relevant to layouting and containers (Measure/Arrange, Collections,..) label Jan 4, 2021
@jeromelaban jeromelaban added the difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. label Feb 15, 2021
@kazo0 kazo0 mentioned this issue May 5, 2021
Merged
6 tasks
@MartinZikmund MartinZikmund added difficulty/challenging 🤯 Categorizes an issue for which the difficulty level is reachable with internals understanding and removed difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. labels Jun 2, 2021
@MartinZikmund MartinZikmund changed the title ItemsRepeater OnElementPrepared is called after NavigationViewItem.OnApplyTemplate ItemsRepeater OnElementPrepared is called after NavigationViewItem.OnApplyTemplate Jun 1, 2023
@MartinZikmund MartinZikmund added the project/navigation-lifecycle 🧬 Categorizes an issue or PR as relevant to the navigation and lifecycle (NavigationView, AppBar, ...) label Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
difficulty/challenging 🤯 Categorizes an issue for which the difficulty level is reachable with internals understanding kind/bug Something isn't working project/layout 🧱 Categorizes an issue or PR as relevant to layouting and containers (Measure/Arrange, Collections,..) project/navigation-lifecycle 🧬 Categorizes an issue or PR as relevant to the navigation and lifecycle (NavigationView, AppBar, ...) project/third-party 3️⃣ Categorizes an issue or PR as relevant to 3rd party libraries
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@MartinZikmund @jeromelaban