~eliasnaur/gio#399: 
Editor InputOp Keys Set is misconfigured

The new Editor InputOp Keys Set listens for Short-UpArrow & Short-DownArrow even though it doesn't do anything with them.

DeleteBackward and DeleteForward may be similar, or just wrong. The Set listens for Short or Shift (both optional), but the code looks for ShortcutAlt or "no modifiers".

Also, a SingleLine editor shouldn't listen for Up or Down arrows at all, I would think. Nor PageUp/Down.

Style gripe: Since the Keys Set uses the literal names of keys (like ↑ and ↓ for NameUpArrow and NameDownArrow), this makes it hard to search for uses of NameUpArrow, etc, e.g. using GoVim's "show references to identifier" function.

Status
REPORTED
Submitter
~theclapp
Assigned to
No-one
Submitted
7 months ago
Updated
7 months ago
Labels
No labels applied.

~theclapp 7 months ago

Also if the Editor is Submit:false, then it shouldn't listen for return/enter.

~theclapp 7 months ago*

https://github.com/gioui/gio seems to be behind https://git.sr.ht/~eliasnaur/gio/tree by 9 commits right now.

When it's up to date, I have a fix for this issue in https://github.com/theclapp/gio.

Right now, if I made a PR, it'd include those commits that I pulled from https://git.sr.ht/~eliasnaur/gio directly.

Feel free to cherry pick the relevant commits out of my fork directly.

~eliasnaur 7 months ago

Elias Naur referenced this ticket in commit 6ddc13c.

~eliasnaur 7 months ago

The new Editor InputOp Keys Set listens for Short-UpArrow & Short-DownArrow even though it doesn't do anything with them.

and

DeleteBackward and DeleteForward may be similar, or just wrong.

Both fixed in latest commit.

The Set listens for Short or Shift (both optional), but the code looks for ShortcutAlt or "no modifiers".

and

Also, a SingleLine editor shouldn't listen for Up or Down arrows at all, I would think. Nor PageUp/Down.

as well as your

Only listen for Short-[C,X] (copy selection and cut selection) when there's a selection.

I'm not sure. I see InputOp.Keys as a static property, describing the complete set of possible shortcuts and not a dynamic set describing what it can handle at any given time. Perhaps you have a use-case where a dynamic set is necessary?

Style gripe: Since the Keys Set uses the literal names of keys (like ↑ and ↓ for NameUpArrow and NameDownArrow), this makes it hard to search for uses of NameUpArrow, etc, e.g. using GoVim's "show references to identifier" function.

Indeed, but I like the short and readable set expressions. Would you also have the modifier names taken form package key?

~theclapp 7 months ago

Both fixed in latest commit.

Thanks!

The Set listens for Short or Shift (both optional), but the code looks for ShortcutAlt or "no modifiers".

Did you quote the right bit? That bit referred to the delete keys, which you said you fixed. Perhaps you meant "Also if the Editor is Submit:false, then it shouldn't listen for return/enter"?

I see InputOp.Keys as a static property, describing the complete set of possible shortcuts and not a dynamic set describing what it can handle at any given time. Perhaps you have a use-case where a dynamic set is necessary?

I'll grant you the Short-[C-X] is arguable.

The bit about skip up/down/page-up/page-down for single-line widgets and skip enter/return for non-submit widgets is so that if you have a single line widget you can get up/down/page-up/page-down yourself, and if you have a non-submit widget you can get enter/return yourself.

In my app I have two places where I have a single-line text widget you can use to filter a list, and in one instance I'd like to scroll the list on page-up/down, and in another instance I'd like to be able to move the selected item in the list on up/down.

I'm on the fence about the submit vs. getting enter/return on your own. It's not like you have to use the submit event to do something with the text in the widget. They seem kind of equivalent, and I've been debating making my text widgets submittable again and just treating it like I do enter/return now. I'm not sure yet if that's the right way to go, for me, but it might be. So that one's arguable too.

I guess it's possible that someone might want to treat enter & return differently?

Indeed, but I like the short and readable set expressions. Would you also have the modifier names taken form package key?

No. To me they're not distinctive enough, that is, I rarely want to search for "ctrl keys". But I frequently want to search for enter/up/down/page-up/page-down keys.

This is also handy when viewing code in a browser. Try searching for ↑ in https://git.sr.ht/~eliasnaur/gio/tree/6ddc13ce6691291f298c6ebbcc08effa7ceb8a91/item/widget/editor.go. I mean, you can, but if you're like me then you had to copy the ↑ from somewhere. Whereas it's a lot easier to search for NameUpArrow.

~eliasnaur 7 months ago

On Tue, 19 Apr 2022 at 15:47, ~theclapp outgoing@sr.ht wrote:

Both fixed in latest commit.

Thanks!

The Set listens for Short or Shift (both optional), but the code looks for ShortcutAlt or "no modifiers".

Did you quote the right bit? That bit referred to the delete keys, which you said you fixed. Perhaps you meant "Also if the Editor is Submit:false, then it shouldn't listen for return/enter"?

I quoted the right bit. Specifically, delete/backspace still listens for shift, not because shift changes their behaviour, but because shift+delete/backspace works in other text fields. It may be a coincidence because other toolkits work differently, and now I re-check shift+delete doesn't do anything here in Firefox/Gmail.

I see InputOp.Keys as a static property, describing the complete set of possible shortcuts and not a dynamic set describing what it can handle at any given time. Perhaps you have a use-case where a dynamic set is necessary?

I'll grant you the Short-[C-X] is arguable.

The bit about skip up/down/page-up/page-down for single-line widgets and skip enter/return for non-submit widgets is so that if you have a single line widget you can get up/down/page-up/page-down yourself, and if you have a non-submit widget you can get enter/return yourself.

In my app I have two places where I have a single-line text widget you can use to filter a list, and in one instance I'd like to scroll the list on page- up/down, and in another instance I'd like to be able to move the selected item in the list on up/down.

I'm on the fence about the submit vs. getting enter/return on your own. It's not like you have to use the submit event to do something with the text in the widget. They seem kind of equivalent, and I've been debating making my text widgets submittable again and just treating it like I do enter/return now. I'm not sure yet if that's the right way to go, for me, but it might be. So that one's arguable too.

I guess it's possible that someone might want to treat enter & return differently?

I see. I still think key sets should be static strings, but for a different reason. Chris (and probably others) have asked for the ability to override Editor shortcuts entirely, not just catch the keys Editor happen to not care about. So I think your use-case is better served as a special-case of the general issue of custom Editor key bindings.

To implement it, Editor should be split into a state object with methods for editing and caret movement, and a widget with an key.InputOp that maps the usual shortcuts. In fact, we could get rid of Editor.Submit entirely and put that logic in the shortcut mapper widget.

Indeed, but I like the short and readable set expressions. Would you also have the modifier names taken form package key?

No. To me they're not distinctive enough, that is, I rarely want to search for "ctrl keys". But I frequently want to search for enter/up/down/page-up/page-down keys.

This is also handy when viewing code in a browser. Try searching for ↑ in https://git.sr.ht/~eliasnaur/gio/tree/6ddc13ce6691291 f298c6ebbcc08effa7ceb8a91/item/widget/editor.go. I mean, you can, but if you're like me then you had to copy the ↑ from somewhere. Whereas it's a lot easier to search for NameUpArrow.

Fair enough. And you can const up = key.NameUpArrow before constructing your key set, which makes it slightly more readable IMO.

Elias

Register here or Log in to comment, or comment via email.