[Accessibility] Use keybindings to add keybindings

[webfiltered]

Adds standard keyboard navigation to the keyboard shortcut editor.

• Enter saves the shortcut
• Escape cancels the edit

Edit: Tab removed.

These keys have intuitive contextual expectations for all users. They are rarely offered as options for key binding in apps, as it is extremely likely to add unnecessary complexity. This will be the case for us, and it will worsen as the app matures and other accessibility concerns are addressed.

N.B. Use of these keys with modifiers is not affected.

┆Issue is synchronized with this Notion page by Unito

[christian-byrne]

Would there be way to map these keys afterwards? For example if someone wants to map Enter to Queue Prompt or Escape to Interrupt?

[huchenlei]

Would there be way to map these keys afterwards? For example if someone wants to map Enter to Queue Prompt or Escape to Interrupt?

VSCode obviously face the same issue. Their solution is to allow user go edit the json file: https://stackoverflow.com/questions/74244703/how-do-i-bind-enter-to-a-command-in-vs-code

I am not sure whether that is the best solution for us though, as the user setting json file is not as accessible in ComfyUI comparing to VSCode.

[huchenlei]

nit: The tab key should be bindable in the desktop app.

[webfiltered]

nit: The tab key should be bindable in the desktop app.

Any specific use case? (Shift) Tab is almost always focus cycling in desktop apps. Text editing is one of the rare exceptions (mainly due to the historic use of tab key.. for tabs).

That said, we can likely ditch the tab handling here - realistically I only implemented it here because I couldn't hit enter to save the shortcut. So I tried to use tab to get to the button - so I started with that.

For example if someone wants to map Enter to Queue Prompt or Escape to Interrupt?

I can see the logic in escape being used on the graph. But again it's a special key in basically every app, this time including most games.. I don't think allowing it to be bound to interrupt queue is good design.

Because it's all of two keys, we could add a special way to handle this for modifier-less escape and enter.

[christian-byrne]

It makes sense but I'm not sure in terms of cost/benefit. It requires making assumptions about user keyboard layout and there will later be kebinding contexts where these keys might make sense to bind

[huchenlei]

I think we shouldn't bind the Tab key to a command by default, but we should not prevent users from binding the tab key.

[webfiltered]

It requires making assumptions about user keyboard layout

Not sure I understand - do you have a specific layout you're concerned about?

there will later be kebinding contexts where these keys might make sense to bind

In my mind, these keys would already be bound to the logical action by default - happy to be corrected, but an example would be great.

[webfiltered]

I think we shouldn't bind the Tab key to a command by default, but we should not prevent users from binding the tab key.

Agreed. Keyboard users simply won't bind them if they're problematic, and whilst we do have other controls on this dialog, enter and escape cover the primary accessibility concerns.

[christian-byrne]

In my mind, these keys would already be bound to the logical action by default - happy to be corrected, but an example would be great

If the user can bind per-context keybindings, there can be contexts where escape or enter do not have a default action but many users want to bind them. One example would be the embedded terminal context, wherein users might want to bind escape to close the terminal.

In general, it's likely that there are already users who bind Enter to the queue prompt action.

[webfiltered]

bind escape to close the terminal

The best way I've seen this kind of thing handled in keyboard-accessible apps is to allow really one-off functions like this as a toggle instead.

In general, it's likely that there are already users who bind Enter to the queue prompt action.

I had thought about this use-case, but I don't believe it will be a large set of users. Ctrl + Enter makes a lot more sense. Using unmodified Enter already overlaps with multi-line text widgets, number input widgets, etc.

Continuing to support this will also create more complexity for us to deal with down the line. If we do decide to go that route, I have a loose UX plan (which I really don't want to implement or be responsible for, but would at least cover some accessibility concerns).

[christian-byrne]

That's fair. I think you're correct afterall -- it's probably better to err on the side of making the dialog fully accessible.