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

How to enable the user to find a TreeViewItem #583

Open
ScottHutchinson opened this issue Sep 25, 2023 · 29 comments
Open

How to enable the user to find a TreeViewItem #583

ScottHutchinson opened this issue Sep 25, 2023 · 29 comments

Comments

@ScottHutchinson
Copy link
Contributor

ScottHutchinson commented Sep 25, 2023

I appreciate any help y'all can give me with this issue.

In my repo (an evolution of my repo that uses the RoseTree, which some of you helped me with about a year ago), in this commit, I have added a Search TextBox to enable the user to enter text so they can find a tree view node whose name or type contains the specified text. In the production version of this repo, the tree sometimes contains thousands of nodes in up to about 13 levels of hierarchy, so I want to give the user a way to find nodes without scrolling through all those nodes.

FIXED 1. After this commit, the search begins as soon as the user starts typing. That will probably be too slow, so how can I change it so it starts searching only when the user clicks Enter?
FIXED 2. Currently, the code selects a matching node only after the user enters too much text and then clicks backspace (e.g., click the 'Expand All' button, then type 'banjoss' in the Search box, then click backspace to find the node containing 'banjos', which the code then selects. Scroll down part of a screen to see the selected node.). Is this an Elmish.WPF bug or just something I'm doing wrong? Possibly implementing item 1 above would fix it.
image

FIXED 3. After clicking backspace, the code selects the matching node, but it doesn't scroll to reveal it if it is off screen. How could I make it scroll to the matching node?
4. If there are multiple matching nodes, the code only selects the last one. How could I make it stop searching after it finds the first or next match? Also, I would be open to other designs if appropriate. For instance, instead of selecting matches, the code could expand the matching portion of the tree. For instance, the user could click the 'Collapse All' button and then search to expand the branches that reveal the matching node. That is how I frequently use Find in Notepad++ when viewing a large XML file with thousands of nodes in a tree.

@TysonMN
Copy link
Member

TysonMN commented Sep 26, 2023

  1. Have two different strings in the model. One is the contents of the text box. When the user clicks enter, copy that string to the second string, which is what is used to do the filtering.

Try that first. Then comes back and let us know if 2, 3, and 4 are still issues for you.

@ScottHutchinson
Copy link
Contributor Author

Sounds good. Thank you. What is the Elmish way to handle the enter key in a textbox? Is there a sample for that?

@ScottHutchinson
Copy link
Contributor Author

ScottHutchinson commented Sep 26, 2023

I found this example on SO, which I think will fit with Elmish. I'll try it.

<TextBox>
  <TextBox.InputBindings>
    <KeyBinding Command="{Binding Path=CmdSomething}" Key="Enter" />
  </TextBox.InputBindings>
</TextBox>

@ScottHutchinson
Copy link
Contributor Author

Progress. This commit resolves item 1 and 2. Please help with items 3 and 4. Thanks!

@TysonMN
Copy link
Member

TysonMN commented Sep 27, 2023

  1. I don't recall ever doing this myself. I asked Google's Bard, and the answer seems good.

https://g.co/bard/share/004e050593ed

Get that working first. Then let's us know if you still haven't solved item 4.

@ScottHutchinson
Copy link
Contributor Author

ScottHutchinson commented Sep 27, 2023

I'm going to try to add a Selected item event handler in C# to call the ScrollToItem method. Not sure how else to do it.

EDIT: Or maybe I could do it using Behaviors like in this sample. But I'll also see if it can be done without Behaviors.

EDIT2: This might work, if the command parameters includes a reference to the ListViewItem.

EDIT3: I got it working like below. Bard answered your question for a list, but that answer was no good for a TreeView.

        // Based on https://stackoverflow.com/a/9494484/5652483
        private void TreeViewSelectedItemChanged(object sender, RoutedEventArgs e) {
            if (sender is TreeViewItem item) {
                item.BringIntoView();
                e.Handled = true;
            }
        }

@ScottHutchinson
Copy link
Contributor Author

This commit partially resolves item 3. But it only works if the selected item's ancestors are all expanded. So I still need to add some code to expand those ancestors.

@ScottHutchinson
Copy link
Contributor Author

ScottHutchinson commented Sep 27, 2023

This commit expands the selected item's ancestors. It works sometimes, but not all the time. I need help. I think I need an OutMsg to update the SearchFoundId field in the model instead of in the unmodifiedModel, but I'm not sure how to do that.

            "IsExpanded" |> Binding.oneWay(fun ((m: Model), { Self = (s: RoseTree<FieldData>) }) -> 
                level < m.ExpandLevel || isAncestorOfFoundField s
            )
            "IsSelected" |> Binding.oneWay(fun ((m: Model), { Self = (s: RoseTree<FieldData>) }) -> 
                let isSelected = isFoundInSearch m.SearchEnterText s.Data
                if isSelected then
                    // TODO: Send a SearchFoundMsg to update the model instead of the unmodifiedModel.
                    unmodifiedModel <- { unmodifiedModel with SearchFoundId = Some s.Data.Id }
                    Debug.WriteLine($"****** Name: {s.Data.Name}, Type: {s.Data.Type} selected")

                isSelected
            )

@TysonMN
Copy link
Member

TysonMN commented Sep 27, 2023

I reformatted my computer recently and don't have it set back up yet.

@marner2 or @LyndonGingerich, can either of you help with this?

@ScottHutchinson
Copy link
Contributor Author

I seem to have one or more use cases where I need to update the model when the "IsSelected" field binding above finds a matching item. But I can't figure out how to make that happen.

@LyndonGingerich
Copy link
Contributor

  1. After this commit, the search begins as soon as the user starts typing. That will probably be too slow, so how can I change it so it starts searching only when the user clicks Enter?

Off the top of my head, the easiest potential solution is probably to set UpdateSourceTrigger on the search TextBox binding.

Reading the thread.

@LyndonGingerich
Copy link
Contributor

This commit expands the selected item's ancestors. It works sometimes, but not all the time. I need help. I think I need an OutMsg to update the SearchFoundId field in the model instead of in the unmodifiedModel, but I'm not sure how to do that.

            "IsSelected" |> Binding.oneWay(fun ((m: Model), { Self = (s: RoseTree<FieldData>) }) -> 
                let isSelected = isFoundInSearch m.SearchEnterText s.Data
                if isSelected then
                    // TODO: Send a SearchFoundMsg to update the model instead of the unmodifiedModel.
                    unmodifiedModel <- { unmodifiedModel with SearchFoundId = Some s.Data.Id }
                    Debug.WriteLine($"****** Name: {s.Data.Name}, Type: {s.Data.Type} selected")

                isSelected
            )

I seem to have one or more use cases where I need to update the model when the "IsSelected" field binding above finds a matching item. But I can't figure out how to make that happen.

No, you're right. There is no way to update the model from a one-way binding. We have to deal with the issue earlier.

If I understand correctly, your problem is that your model is not always consistent: SearchFoundId is not always up-to-date with the results of isFoundInSearch. Thus, wherever you are using SearchFoundId, it does not contain all the information you need.

Normalizing data is usually a good idea. Have you considered removing SearchFoundId from the model and instead searching the tree using isFoundInSearch? This would remove the possibility of data inconsistency (possibly at the cost of performance). Or you could update SearchFoundId in your update function whenever Model.SearchEnterText or any RoseTree.Data is set (caching the computation results, which is famously difficult).

@LyndonGingerich
Copy link
Contributor

You're more experienced than me, but I would suggest avoiding a mutable model identifier if at all possible. That feels like reimplementing Elmish. If you're wanting to avoid triggering binding updates, you can do that with Binding.addLazy.

@ScottHutchinson
Copy link
Contributor Author

ScottHutchinson commented Oct 2, 2023

FIXED in this commit also getting this error occasionally.

Exception thrown: 'System.InvalidOperationException' in System.Core.dll
System.Windows.Data Error: 8 : Cannot save value from target back to source. BindingExpression:Path=IsSelected; DataItem='DynamicViewModel`2' (HashCode=34416674); target element is 'TreeViewItem' (Name=''); target property is 'IsSelected' (type 'Boolean') InvalidOperationException:'System.InvalidOperationException: Property path is not valid. 'System.Dynamic.DynamicObject+MetaDynamic' does not have a public property named 'Items'.
   at CallSite.Target(Closure , CallSite , Object , Object )
   at System.Dynamic.UpdateDelegates.UpdateAndExecuteVoid2[T0,T1](CallSite site, T0 arg0, T1 arg1)
   at MS.Internal.DynamicPropertyAccessorImpl.SetValue(Object component, Object value)
   at MS.Internal.Data.PropertyPathWorker.SetValue(Object item, Object value)
   at System.Windows.Data.BindingExpression.UpdateSource(Object value)'

@LyndonGingerich
Copy link
Contributor

This line must be the issue somehow. It's a TreeViewItem style, it's setting IsSelected, and it's setting it to binding IsSelected.

@LyndonGingerich
Copy link
Contributor

By the way, you might try the new view model classes if you want better error messages. It'll tell you what view model class the problem binding is on.

@LyndonGingerich
Copy link
Contributor

It's a one-way binding and the error message says it couldn't set the value back to the source. What if you tried <Setter Property="IsSelected" Value="{Binding IsSelected, Mode=OneWay}" />?

@ScottHutchinson
Copy link
Contributor Author

It's a one-way binding and the error message says it couldn't set the value back to the source. What if you tried <Setter Property="IsSelected" Value="{Binding IsSelected, Mode=OneWay}" />?

Makes sense. I added Mode=OneWay as @LyndonGingerich suggested, and the errors went away. I need to learn more about all these binding modes.

@ScottHutchinson
Copy link
Contributor Author

By the way, you might try the new view model classes if you want better error messages. It'll tell you what view model class the problem binding is on.

@LyndonGingerich That looks promising. I will try it soon. Thanks!

@ScottHutchinson
Copy link
Contributor Author

Issues still to be resolved:
Item 4 at the start of this thread.
5. The 'Collapse All' button does not work following a search. The search expands the ancestors of the found item, but leaves the ExpandLevel field in the model at 0. Then the CollapseAll update message sets the ExpandLevel field = 0, which does nothing. I have tried two different fixes, but neither worked out.
6. The search automatically expands the ancestors of the found item only if the user has previously expanded and collapsed the tree.

@LyndonGingerich
Copy link
Contributor

I need to learn more about all these binding modes.

The basic four are pretty simple:

  1. A one-way binding exposes a getter and no setter.
  2. A one-way-to-source binding exposes a setter and no getter. (In my experience, people often use command bindings when one-way-to-source would be simpler.)
  3. A two-way binding exposes both a getter and a setter.
  4. A command binding exposes an ICommand.

At least, this is my understanding from working with aforementioned view model classes.

@LyndonGingerich
Copy link
Contributor

  1. The 'Collapse All' button does not work following a search. The search expands the ancestors of the found item, but leaves the ExpandLevel field in the model at 0. Then the CollapseAll update message sets the ExpandLevel field = 0, which does nothing. I have tried two different fixes, but neither worked out.

What does ExpandLevel represent? For example, if I have parent elements A, B, and C, where A is expanded, B is not expanded, and C has a descendant two levels below it expanded, what is the value of ExpandLevel?

@LyndonGingerich
Copy link
Contributor

  1. If there are multiple matching nodes, the code only selects the last one. How could I make it stop searching after it finds the first or next match? Also, I would be open to other designs if appropriate. For instance, instead of selecting matches, the code could expand the matching portion of the tree. For instance, the user could click the 'Collapse All' button and then search to expand the branches that reveal the matching node. That is how I frequently use Find in Notepad++ when viewing a large XML file with thousands of nodes in a tree.

A hypothesis:

You set selection using TreeViewItem.IsSelected bound to IsSelected in your view model. The IsSelected view model binding uses isFoundInSearch, which looks as though it can return true on multiple items. The default value of TreeView.SelectionMode is Single. Therefore, whenever IsSelected is set on a TreeViewItem, it probably sets IsSelected to the previous selected item to false, resulting in only the last found item being selected.

From my understanding, single selection is the behavior you desire. You should either change isFoundInSearch so that it returns true on only one item or use one of the TreeView selection properties. (While the properties list of TreeView does not list SelectedValue and SelectedValuePath, which are the preferred method of setting selection, this article illustrates using them on TreeView in WPF, so they are probably worth a try.)

@LyndonGingerich
Copy link
Contributor

  1. The search automatically expands the ancestors of the found item only if the user has previously expanded and collapsed the tree.

Is the issue in the view or the view model? Can you tell whether IsExpanded is set correctly on the view model?

@ScottHutchinson
Copy link
Contributor Author

What does ExpandLevel represent?

@LyndonGingerich The code below from here shows how ExpandLevel is used to determine if an item's isExpanded property is true or false. Initially, the ExpandLevel defaults to 2, so levels 0 and 1 are expanded while deeper levels farther from the root are collapsed. The user can increase or decrease the ExpandLevel using four different buttons. That all worked perfectly, until I introduced the Search text box, which expands an item's ancestors without changing the ExpandLevel.

            "IsExpanded" |> Binding.oneWay(fun ((m: Model), { Self = (s: RoseTree<FieldData>) }) -> 
                level < m.ExpandLevel || isAncestorOfFoundField s
            )

@LyndonGingerich
Copy link
Contributor

  1. The search automatically expands the ancestors of the found item only if the user has previously expanded and collapsed the tree.

Is the issue in the view or the view model? Can you tell whether IsExpanded is set correctly on the view model?

Still waiting for a response to this.

@LyndonGingerich
Copy link
Contributor

Any update?

@ScottHutchinson
Copy link
Contributor Author

Sorry, I had to put this on the back burner for now while I work on some higher priority tasks. I'll work on it again soon.

@LyndonGingerich
Copy link
Contributor

No worries, just making sure you're not waiting for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants