NavigationView selection change and child items

[marcelwgn]

When changing the selection state through the API, the newly selected item is not always visible, and might sometimes be hidden if it is a child item of a NavigationViewItem.

Should the behavior change in that regard? Should changing the selected item also result in the newly selected item being visible and on screen?

See this PR for more context: microsoft/WinUI-Gallery#478

[Felix-Dev]

This issue has been mentioned in #2845 and in my opinion should be fixed. I expect the selected item to show up as such in the NavigationView pane.

[StephenLPeters]

Sounds like we need to scroll into view the selected item? I think this is what list view does for instance.

[marcelwgn]

Yes in addition to scrolling the selected item into view, we also need to expand the necessary NavigationViewItems, which might be a bit problematic, if the NavigationViewItems don't know their parent items.

[StephenLPeters]

When the navigationViewItem raises its selected changed event we can have the parent navigation view expand the tree it is in right?

[marcelwgn]

Yes, the parent NavigationView could or rather should do that. Does the NavigationView know the parent of each of the NavigationViewItems? Is that the same as the VisualTreeParent?

[YuliKl]

I feel like @ojhad and I had discussed this scenario, but it's been too long and I can't recall the details 😕

I think I was more focused on making each item aware of whether it was expanded or had a selected descendent. I don't believe that we added any such tracking to NavigationView itself. So items have an IsChildSelected property, but no knowledge about their ancestors.

In XCG, we should be able to keep track of the newly selected item's parent and expand the correct subtree using the parent's IsExpanded property. Is there a straight-forward mechanism for bringing the newly-selected item into view? If there is, I think we can fix the strange behavior I was seeing in XCG.

[StephenLPeters]

I feel like @ojhad and I had discussed this scenario, but it's been too long and I can't recall the details 😕

I think I was more focused on making each item aware of whether it was expanded or had a selected descendent. I don't believe that we added any such tracking to NavigationView itself. So items have an IsChildSelected property, but no knowledge about their ancestors.

In XCG, we should be able to keep track of the newly selected item's parent and expand the correct subtree using the parent's IsExpanded property. Is there a straight-forward mechanism for bringing the newly-selected item into view? If there is, I think we can fix the strange behavior I was seeing in XCG.

UIElement.BringIntoView() should do the trick. I think this is a bug in NavView though, I feel like you shouldn't need to call that.

[marcelwgn]

@ranjeshj @StephenLPeters Would you be fine with me working on this issue?

[StephenLPeters]

That would be great @chingucoding !

[marcelwgn]

I think we need to rethink this a bit more. If an item has not been realized (which is the case quite often as we might recycle them), the NavigationView and the NavigationViewItems are disconnected meaning that you can tell either side that the NVI is selected, yet beside saying "yup the NVI is selected", they can't do much more.

We have no fast way to determine the parent of a NVI. We could ask every on level 0 if they have their children selected, and ask the one with a selected child which of it's children is selected, and so on, but this would be quite expensive.

So the question now is how do we deal with this? When items are not realized, APIs like FrameworkElement.Parent don't work.
No matter what solution we might choose, it will probably require a lot of work. We will need to tell every NVI (realized or not) what NavigationView or NavigationViewItem it reports to in case of selection change, and similarly, the NavigationView or NavigationViewItem needs to be able to get a path of all NavigationViewItems from layer 0 down to the selected item.

[ranjeshj]

Trying to catch up to this conversation. Is the intention that the selected item should always visible. This is currently not possible through the UI, since you have to bring the item into view in order to select it but possible if done programatically (like the case you mention with search)

Changing selection programmatically causing expanding and bringing items in view sounds a like a lot of side effects. I wonder if this needs to be a separate API (like RealizeAndBringIntoView that takes a dataItem or a NVI). That way you can programmatically set selection and optionally bring that into view.

There are two different issues being discussed if I understand correctly.

1. What happens when you programmatically select an item that is not under a fully expanded tree.
2. Should currently selected item (or its parent) be always visible.

For (1) If a child is programmatically selected, selection model should reflect that and the parent chain should react appropriately. If the child is collapsed, the parent will show as selected (same as what happens when you manually select a child and collapse its parent). If the child is not in the parent chain, we should update it when that child enters the tree, not when the IsSelected property changes (since we don't know the child's index path to update selection model)

[marcelwgn]

That's a very good point you are raising here. Changing selection should probably not have those side effects. Having a separate API for this is probably the better solution here, that way we don't force so many UI updates on simple operations such as selection change.

Regarding the IsSelected what would the expected behavior for #3149 be then? Only update the SelectedItem once that item get's added to the visual tree? What if multiple items with IsSelected set to true show up in the visual tree? Is it a first come first serve or will the latest that has IsSelected set to true be the selected item?

[Felix-Dev]

@ranjeshj

Should currently selected item (or its parent) be always visible.

What definitely should work is the following issue: #2814

I would look into that issue but it seems to me @chingucoding is also focusing on that area for now and he perhaps plans to fix that.

[marcelwgn]

This issue and #2814 are orthogonal I think and have little overlap. Given that this issue might need a bit more consideration, I would say, feel free to work on #2814 .

[ranjeshj]

@Felix-Dev I agree that #2814 is a bug and likely orthogonal to this. After switching to top, the parent should show selected. If we are using the same selection model, I wonder why it does not show up selected already. In any case, that sounds like a bug.

@chingucoding I would say last one with IsSelected=true added to the tree wins. That is likely the simplest to implement and likely cover most scenarios. @anawishnoff and @YuliKl can pitch in if they feel otherwise.

[YuliKl]

I would say last one with IsSelected=true added to the tree wins.

@ranjeshj I'm sorry, I'm not quite following. What do you mean by the last item added to the tree?

[marcelwgn]

The last item added to the tree would essentially mean, the last item we got to render. For example:

• Item 1
• Item 2 (Collapsed)
  • Item 3 (IsSelected=true)
  • Item 4
  • Item 5 (IsSelected=true)

If we expand Item 2, the last item added to the tree with IsSelected=true would be Item 5, thus this will be the selected item. All other items which have IsSelected item set to true will be ignored.

[YuliKl]

Oh, I see, thank you for the explanation @chingucoding! NavigationView doesn't support multiselect as a concept, but IsSelected is a property on the item and we're not enforcing any sort of exclusivity.

Yes, this approach sounds good to me.

[marcelwgn]

@Felix-Dev I assume you would like to work on #2814 right? To prevent merge conflicts, would you like me to wait with #3149 or would you like to tackle that too?

[StephenLPeters]

Oh, I see, thank you for the explanation @chingucoding! NavigationView doesn't support multiselect as a concept, but IsSelected is a property on the item and we're not enforcing any sort of exclusivity.

Yes, this approach sounds good to me.

I think we do enforce exclusivity, but we can't while the item is not realized. Both of these elements are getting realized at the "same" time and thus we have to pick one. So in Marcel's case the IsSelected property of Item3 would be set to false. @chingucoding to make sure I'm remembering this correctly?

[marcelwgn]

Setting IsSelected on multiple items in XAML leaves us with the multiple items having IsSelected set to true. The same does not happen if you set IsSelected to true on multiple items in code behind. In that case it's last man standing. So in the example, as Stephen correctly said, we would set IsSelected to false on Item3.

[Felix-Dev]

@chingucoding

I assume you would like to work on #2814 right? To prevent merge conflicts, would you like me to wait with #3149 or would you like to tackle that too?

Hmm, for #2814, at least in data-binding mode, I would also need to find the top-level parent of a selected child (as only the top-level parent is shown in the pane).

So if I have something like this:

• Item 1
  [...]
• Item 10 (in overflow menu in Top mode typically)
  • Item 10.1 (selected)

and then switch from Left mode to Top pane display mode, I need to show Item 10 as the right-most top-level item left to the overflow menu.

I could iterate through the NavViewItem containers available and check if they are a top-level item and if NavVigationViewItem.IsChildSelected is true. But...what happens if virtualization kicks in and there are currently no item containers for Item 10 and its child Item 10.1? I'm not sure right now how to show Item 10 in the Top pane then as I have no item container to work with here.

Reading through the discussion here, I believe the same problem is being discussed here:

For (1) If a child is programmatically selected, selection model should reflect that and the parent chain should react appropriately. If the child is collapsed, the parent will show as selected (same as what happens when you manually select a child and collapse its parent). If the child is not in the parent chain, we should update it when that child enters the tree, not when the IsSelected property changes (since we don't know the child's index path to update selection model)

It appears to me whatever is decided here and for issue #3149 would also apply to how issue #2814 will be handled then. As such, if I'm not mistaken, I think issue #2814 should be handled together together with these issues you have reported.

[marcelwgn]

I am not sure if I correctly understood the issue, but let me try to recap here:

When switching from left to top, we would like the top most parent of the selected item be present in the list of visible items and be selected And for that to work, we need to figure a way out to find the parent of a selected item which might not be realized.

Regarding Item 10 not being realized, you can "force" create that item, after all we NEED that to be realized if we want to show it being selected. And given that the overflow menu already seems to have some logic in that regard, I think this is not the difficult part of #2814 .

The main pain point I see here is that for unrealized items, we don't really have a way to link them to their parent. They are not aware of their parent NavigationView nor their parent NavigationViewItem.

I totally agree with your assessment that we those probably need to be tackled together. However I currently don't know how we would fix this issue with the unknown parents. Realizing all items to walk their children, and their children and so on, doesn't sound that good of a idea as this could be very expensive if we are talking about hundred of items.

[StephenLPeters]

Why can't we just determine the selected item via the SelectionModel.SelectedIndex?

[marcelwgn]

I think the issue I noticed was that we know the new selected item, however the SelectedIndex isn't up do date in a lot of the areas that get called when the selection changes, e.g. when showing the animation.

[StephenLPeters]

The selected index should be up to date while we transition from left nav to top nav thought right? So we can use that to manually devirtualize the correct nodes

[github-actions]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 5 days.