ListView collection navigator not overridable

[danielmarbach]

It took as a while to understand that key strokes on a list view are always intercepted and forwarded to the collection navigator. Unfortunately this means when I want to intercept key strokes on a list view in a different way I have no possibility to do so but have to fall back to the application level key down events.

The collection navigator also looses the actual Key information while doing so which means on the search string I can't different a 'b' from a 'B' as an example.

If the collection navigator would be overridable I could at least return one that never gets a matching item by always returning -1 which then would return false on the OnKeyDown and therefore raising the KeyDown event if I understand correctly.

[tznind]

This issue overlaps with:

• Revisit v2 Cancelable Event Pattern - was KeyDown and MouseEvent no longer fires for Listbox #3950

[YourRobotOverlord]

I would +1 this as a feature request, for TreeView as well.
I ran into this whilst looking for a way to add icons to ListView items and realized that prepending them to a string broke the built-in list navigation, which works wonderfully.
I also ran into this while trying to implement some custom navigators such as "j4" = down 4 lines

🤔 Implementing this in TG seemed simple at a glance:

CollectionNavigator.cs

- public class CollectionNavigator : CollectionNavigatorBase
+ public class CollectionNavigator : CollectionNavigatorBase, ICollectionNavigator

ICollectionNavigator.cs

+ using System.Collections;
+ 
+ namespace Terminal.Gui;
+ public interface ICollectionNavigator
+ {
+ IList Collection { get; set; }
+ public int GetNextMatchingItem (int currentIndex, char keyStruck);
+ public event EventHandler<KeystrokeNavigatorEventArgs> SearchStringChanged;
+ }

ListView.cs

- public CollectionNavigator KeystrokeNavigator { get; } = new ();
+ public ICollectionNavigator KeystrokeNavigator { get; init; } = new CollectionNavigator();

TreeView.cs

- public CollectionNavigator KeystrokeNavigator { get; } = new ();
+ public ICollectionNavigator KeystrokeNavigator { get; init; } = new CollectionNavigator();

[tznind]

There is a lot of complexity in collection navigator and different subclasses for each view type that uses the abstract base.

The most common things to want to change are probably these two methods:

public static bool IsCompatibleKey (Key a)
private bool IsMatch (string search, object value)

Easiest way to give user control of those could be to make a sub property e.g. ISearchStrategy (Or similar) that has only these two methods.

Then code becomes

myView.CollectionNavigator.SearchStrategy = new MyStrategy()

[tznind]

PR is ready for review, guys please feel free to check it out and let me know if it does not go far enough for your needs.

If the collection navigator would be overridable I could at least return one that never gets a matching item by always returning -1 which then would return false on the OnKeyDown and therefore raising the KeyDown event if I understand correctly.

You can now do this with:

class NeverMatcher : ICollectionNavigatorMatcher
{
    /// <inheritdoc />
    public bool IsCompatibleKey (Key a) { return false; }

    /// <inheritdoc />
    public bool IsMatch (string search, object value) { throw new NotSupportedException (); }
}

[...]

ListView lv = new ListView { Source = new ListWrapper<string> (source) };
lv.KeystrokeNavigator.Matcher = new NeverMatcher ();

Unfortunately this means when I want to intercept key strokes on a list view in a different way I have no possibility to do so

There is a seperate issue which is that we want to prioritise user key down event handlers over keystroke navigation, but that is best handled seperately and on a 'whole repo' scope (see #3950)

[tig]

Here's an updated test that ensures the entire keydown flow is processed correctly:

    [Fact]
    public void CollectionNavigatorMatcher_KeybindingsOverrideNavigator ()
    {
        Application.Top = new ();

        ObservableCollection<string> source = new () { "apricot", "arm", "bat", "batman", "bates hotel", "candle" };
        ListView lv = new ListView { Source = new ListWrapper<string> (source) };

        Application.Top.Add (lv);
        lv.SetFocus ();

        lv.KeyBindings.Add (Key.B, Command.Down);

        Assert.Equal (-1, lv.SelectedItem);

        // Keys should be consumed to move down the navigation i.e. to apricot
        Assert.True (Application.RaiseKeyDownEvent (Key.B));
        Assert.Equal (0, lv.SelectedItem);

        Assert.True (Application.RaiseKeyDownEvent (Key.B));
        Assert.Equal (1, lv.SelectedItem);

        // There is no keybinding for Key.C so it hits collection navigator i.e. we jump to candle
        Assert.True (Application.RaiseKeyDownEvent (Key.C));
        Assert.Equal (5, lv.SelectedItem);

        Application.Top.Dispose ();
        Application.ResetState ();
    }

This test fails currently.

Here's the fix to ListView:

    /// <inheritdoc/>
    protected override bool OnKeyDown (Key key)
    {
        // If marking is enabled and the user presses the space key don't let CollectionNavigator
        // at it
        if (AllowsMarking)
        {
            IEnumerable<Key> keys = KeyBindings.GetAllFromCommands (Command.Select);

            if (keys.Contains (key))
            {
                return false;
            }

            keys = KeyBindings.GetAllFromCommands ([Command.Select, Command.Down]);

            if (keys.Contains (key))
            {
                return false;
            }

        }

+        // If the key was bound to a command, let normal KeyDown processing happen. This enables overriding the default handling.
+       // See: https://github.com/gui-cs/Terminal.Gui/issues/3950#issuecomment-2807350939
+       if (KeyBindings.TryGet (key, out _))
+       {
+            return false;
+       }
+
        // Enable user to find & select an item by typing text
        if (CollectionNavigatorBase.IsCompatibleKey (key))
        {
            int? newItem = KeystrokeNavigator?.GetNextMatchingItem (SelectedItem, (char)key);

            if (newItem is { } && newItem != -1)
            {
                SelectedItem = (int)newItem;
                EnsureSelectedItemVisible ();
                SetNeedsDraw ();

                return true;
            }
        }

        return false;
    }

[tznind]

Here's the fix to ListView

Thanks, I have applied

[tznind]

@tig thanks for this fix, I have also applied it to TableView and TreeView along with relevant tests - in #4029

[danielmarbach]

@tznind wow that was quick 😍 Thank you!