Skip to content

Conversation

@Dwedit
Copy link
Contributor

@Dwedit Dwedit commented May 25, 2019

Attempts to fix Issue #1

First off, I have never used WPF before, and don't really have any clue what I'm doing, but I found some situations where the Tree control can lose track of where the selection or cursor is, and it snaps back to the first item instead of where the selection is.

So I detected the cases where it loses track (container == null), and call FocusNode on the selected node, and somehow, that fixes the issue.

I also simplified the code, refactoring out some copy-pastes.

I don't know why it works, but it fixes keyboard control whenever I try to use it.

…ode (selected node) if it loses track of the tree node.
@trigger-segfault
Copy link
Owner

I do recall having a headache working with the tree node system navigation. The ICSharpCode.TreeView library that this is optimized from is frustratingly complicated, and I still barely understand its node balancing system.

Let me just compare the changes for any edge cases because there are a lot of weird scenarios.

In the future, it's recommended to conform to the coding standards of the project when submitting PRs. But this is partially my fault for not using the default C# standards or having an .editorconfig file.

@trigger-segfault
Copy link
Owner

Do you have steps to reproduce the issue? Just to help figure out when these situations are that need to be checked. If a certain file structure is required then I can always construct it myself.

@Dwedit
Copy link
Contributor Author

Dwedit commented May 27, 2019

When it loses track of the item, Then the TreeView's OnKeyDown event will not have a FileTreeViewItem as e.OriginalSource, instead, it will have the entire FileTreeView control itself as e.OriginalSource. In such a state, if you press a direction key on the keyboard, it will reset the position back to the top. I figured out that if you called FocusItem, it would restore the proper position, and a single selected item could be used to easily determine which item to focus.

I think I found one mistake in my code, I had removed the selection count checks on Space and Enter, those should be put back in. If it doesn't lose track, then the selection count can be something other than 1.

As for what kind of directory would trigger the issue, I could reproduce it on anything big and complicated, such as my Windows directory. I think it loses track of the item when the tree periodically updates in the background.

Maybe it should save and restore the currently focused item when it alters the tree in any way. But the FileTreeView control has no FocusedItem property at all, or any other way to easily find which item is focused.

@trigger-segfault
Copy link
Owner

I've managed to reproduce the issue. But I've actually figured out the very root cause of this. I optimized the FileNodeViewModelCollection to not raise a collection Reset when populating a node's children during expansion. Instead it adds them, sorts them, then raises a collection Add. Other places have been similarly optimized.

When the collection reset was happening, focus was being diverted away from the selected node, thus shifting focus to the actual tree view.

As for focus properties, that is determined by WPF utilities like FocusManager, you can use this to find what currently has keyboard focus.

I need to make a few changes, then this will be good to merge tomorrow. Nice work.

* Spaces to Tabs
* Braces start on same line.
Switching to `selectedItem` certainly made things look nicer. A lot of the redundant checks can be sent somewhere else.

* Reordered getting of `selectedItem`. Re-add `ItemsControl.ItemsControlFromItemContainer(container)`.
* Re-added `(Keyboard.Modifiers == ModifierKeys.None && SelectedItems.Count == 1)` check to Activate item.
* Added some descriptive comments.
@trigger-segfault trigger-segfault merged commit 1cf706e into trigger-segfault:master May 28, 2019
DavidKarlas pushed a commit to DavidKarlas/WinDirStat.Net that referenced this pull request Mar 28, 2025
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

Successfully merging this pull request may close these issues.

2 participants