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

Support "Auto Move To Next Item" in ListView #1962

Closed
tig opened this issue Aug 25, 2022 · 6 comments · Fixed by #1966
Closed

Support "Auto Move To Next Item" in ListView #1962

tig opened this issue Aug 25, 2022 · 6 comments · Fixed by #1966

Comments

@tig
Copy link
Collaborator

tig commented Aug 25, 2022

ListView supports multi-select; SPACE toggles (@tznind I don't see a sample of TableView demonstrating multi-select).

In Out-ConsoleGridView a user has requested that when in multi-select mode (-OutputMode Multiple) pressing SPACE to select will toggle the item and move to the next row: PowerShell/ConsoleGuiTools#148

I don't see how to implement this in a generic/extensible way.

Ideally, I'd be able to (in Out-ConsoleGridView) define a new Command be added named ToggleCheckedWithKey:

...
            if (!_applicationData.MoveToNextOnSelect)
            {
                var cmd = new Command();
                _listView.AddCommand(cmd, () => { if (_listView.MarkUnmarkRow ()) return _listView.MoveDown (); else return false;  });
                _listView.AddKeyBinding(Key.Space, cmd);
            }

            win.Add(_listView);
...

But Command is not public, so this won't work.

We could modify ListView:

// change this:
			AddCommand (Command.ToggleChecked, () => MarkUnmarkRow ());

// to this:

			AddCommand (Command.ToggleChecked, () => {
				if (MarkUnmarkRow ()) {
					if (_moveNextOnToggleWithKey) {
						return MoveDown ();
					}
					return true;
				} else {
					return false;
				}
			});

But this means that if Command.ToggleChecked is invoked in a way OTHER than the SPACE key the MoveDown will happen, which is not what we want. Also, we have all this Command and KeyBinding functionality, and it seems crazy to have to add yet-another-option to ListView.

What are the best ideas for addressing this?

@tznind
Copy link
Collaborator

tznind commented Aug 25, 2022

Ideally, I'd be able to (in Out-ConsoleGridView) define a new Command be added named ToggleCheckedWithKey

Most Commands are named after the effect on the control/application rather than the triggering action (key press) so a better command name would be ToggleCheckedAndMoveNext (although I'm open to suggestions on the name of this, ToggleCheckedAndNext would be shorter or ToggleCheckedThenNext).

  • For a ListView this would move to next item.
  • For a checkbox this would move focus to next control
  • For a radio button... probably the same as checkbox

If we were to implement this it would mean a user could bind ToggleChecked to space and ToggleCheckedAndMoveNext to shift+space.

If API user wants it to always move next on check then they can just rebind the key for ToggleChecked to ToggleCheckedAndMoveNext.

What do you think?

@tznind
Copy link
Collaborator

tznind commented Aug 25, 2022

Another option I guess would be to expand keybinding so you could send multiple commands e.g.

myview.AddKeybinding(Key.space | Key.shiftmodifier,new []{Command.ToggleChecked, Command.Down)}

Above is pseudocode for how user might call this

Not sure how hard that would be but might be more elegant and versatile?

@tig
Copy link
Collaborator Author

tig commented Aug 25, 2022

Another option I guess would be to expand keybinding so you could send multiple commands e.g.

myview.AddKeybinding(Key.space | Key.shiftmodifier,new []{Command.ToggleChecked, Command.Down)}

Above is pseudocode for how user might call this

Not sure how hard that would be but might be more elegant and versatile?

I half-considered this when typing the above. I think it's an intriguing idea.

The implications are huge, though: Chaining commands is the camel's-nose-under-the-tent of building Turing complete scripting system. LOL. Before long peeps would be asking for a "delay command" and "conditional commands" etc... So we should tread carefully in this direction. Since you know the Command implementation better than I, how hard do you think it would be to implement simple chaining of commands as you suggest?

@tig
Copy link
Collaborator Author

tig commented Aug 25, 2022

Ideally, I'd be able to (in Out-ConsoleGridView) define a new Command be added named ToggleCheckedWithKey

Most Commands are named after the effect on the control/application rather than the triggering action (key press) so a better command name would be ToggleCheckedAndMoveNext (although I'm open to suggestions on the name of this, ToggleCheckedAndNext would be shorter or ToggleCheckedThenNext).

  • For a ListView this would move to next item.
  • For a checkbox this would move focus to next control
  • For a radio button... probably the same as checkbox

If we were to implement this it would mean a user could bind ToggleChecked to space and ToggleCheckedAndMoveNext to shift+space.

If API user wants it to always move next on check then they can just rebind the key for ToggleChecked to ToggleCheckedAndMoveNext.

What do you think?

When I started writing this Issue, I actually had it titled "All Views should support 'ToggleAndMoveNext`". So I'm a fan of this concept.

@tig
Copy link
Collaborator Author

tig commented Aug 25, 2022

To summarize ideas so far:

  1. Expose AddCommand as public and enable users to extend the commands a View supports.
  2. Define a set of new Commands for ToggleCheckedThenNext and ToggleCheckedThenPrevious.
  3. Enable Commands to be chained; enhancing AddKeyBinding to take a list of Command objects that will be executed in succession.

I'd like to have a discussion as to why #1 might be a bad idea. I haven't thought enough about it, but I have some instinct that says it might be bad. But at the same time I like it.

@tznind
Copy link
Collaborator

tznind commented Aug 25, 2022

Regarding 1

Expose AddCommand as public and enable users to extend the commands a View supports.

AddCommand is currently protected. I didn't want users to confuse the keybinding system with the KeyPress events system.

I had intended AddCommand to be used by when defining a new View Type (subclassing View or Checkbox something). It lets the programmer say 'this view supports Toggling and heres how'.

KeyPress is the public face of the View and lets the user define arbitrary new behaviour. The programmer doesn't need to get as deeply into gui.cs to use this feature and it is immediately recognizeable pattern.

That said a public the user can still do something like:

myView.ClearKeybinding (Command.ToggleChecked);
myView.KeyPress += (e)=>
{
   // switch toggle state
  // move next item
};

Also a (maybe not very good) reason its protected is to reduce the public surface of the keybinding system in case of breaking changes in future (e.g. changing Func<bool?> f to CommandDelegate or something else).

Regarding 2 and 3

I will take a look at chaining commands, I think it will be quite easy and should have quite a small footprint. Thinking about it more now, Im not sure having a new Command that is implicitly 2 operations joined together (ToggleCheckedThenNext) is the best approach if we can get what we want relatively easily with chaining.

The more Enum values in Command the harder it becomes to implement the set in new views (including people subclassing View in their own programs).

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

Successfully merging a pull request may close this issue.

2 participants