enhance(ux): keymap manager for plugins

[xyhp915]

• enhance(ux): render the keymap tables pane into a modal dialog
• feat: bind custom shortcuts for the plugin registered shortcut

Demo

CleanShot.2023-05-18.at.18.01.54.mp4

[cannibalox]

beautiful !

about shortcut conflicts : a lot of apps (eg adobe or autodesk products) have a way to navigate back and forth between the conflicting shortcut(s) to let the user resolve issues (in the example from 0:35, it could be really useful to quick jump to the assigned shortcut for :go/whiteboard, in case the user wants to keep g w for vim shortcuts and change go whiteboard to something else). Maybe a clickable link in the alert box ?
or in the future, introduce a search box to filter the ever-increasing list of results (or just quick jump to result) ?
also maybe in the long run, it could be nice to expand the command palette ui to enable returning+editing shortcuts directly from the results list ?

you probably already have this on your radar though, so feel free to disregard...
This new modal is a great improvement :)

[xyhp915]

beautiful !

about shortcut conflicts : a lot of apps (eg adobe or autodesk products) have a way to navigate back and forth between the conflicting shortcut(s) to let the user resolve issues (in the example from 0:35, it could be really useful to quick jump to the assigned shortcut for :go/whiteboard, in case the user wants to keep g w for vim shortcuts and change go whiteboard to something else). Maybe a clickable link in the alert box ? or in the future, introduce a search box to filter the ever-increasing list of results (or just quick jump to result) ? also maybe in the long run, it could be nice to expand the command palette ui to enable returning+editing shortcuts directly from the results list ?

you probably already have this on your radar though, so feel free to disregard... This new modal is a great improvement :)

Hi @cannibalox, Highly appreciate your concern and valuable advice; the current UX with shortcut conflicts is really not friendly enough. Fortunately, we'll address this in another refactoring for the shortcuts system as soon as possible!

[Bad3r]

Great enhancement! I tested this PR locally; here are some of my observations;

• when opening the keymap manager from the command palette, Logseq UI reloads/flashes quickly. It's not an issue, but it's weird that the whole UI is refreshed, and it's noticeable.
• After opening the keymap manager, it's not possible to use arrow keys to navigate up/down
• The closing 'x' scrolls down and overlaps with other UI elements; you might want to consider making it fixed at the top
  

[logseq-cldwalker]

@xyhp915 Looks nice! 👍 🚢 Works well on openai plugin. Left some minor questions

[logseq-cldwalker]

when opening the keymap manager from the command palette, Logseq UI reloads/flashes quickly. It's not an issue, but it's weird that the whole UI is refreshed, and it's noticeable.
After opening the keymap manager, it's not possible to use arrow keys to navigate up/down
The closing 'x' scrolls down and overlaps with other UI elements; you might want to consider making it fixed at the top

I didn't see the first behavior on osx but I did see the other two

[xyhp915]

Great enhancement! I tested this PR locally; here are some of my observations;

• when opening the keymap manager from the command palette, Logseq UI reloads/flashes quickly. It's not an issue, but it's weird that the whole UI is refreshed, and it's noticeable.
• After opening the keymap manager, it's not possible to use arrow keys to navigate up/down
• The closing 'x' scrolls down and overlaps with other UI elements; you might want to consider making it fixed at the top
  

Nice shot to UI details. I'll simply polish the UI for this PR :-). Because we are unifying the entire UI component design system, the design of this modal component will also be improved.

[logseq-cldwalker]

Just to clarify I'm asking if it's a breaking change, not when we should do work to make it backwards compatible. Is it a breaking change? If so, we should mention it in the PR title as users have complained when we don't communicate breaking changes. If it's a minor we can call it "minor behavior change" like we did in #9291

…

On Fri, 26 May 2023 at 10:31, Charlie ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/main/frontend/modules/shortcut/config.cljs <#9430 (comment)>: > @@ -113,19 +113,19 @@ :whiteboard/zoom-out {:binding "shift+dash" :fn #(.zoomOut (.-api ^js (state/active-tldraw-app)) false)} - :whiteboard/zoom-in {:binding "shift+=" + :whiteboard/zoom-in {:binding "shift+equals" I think it's best to support them in later refactorings. — Reply to this email directly, view it on GitHub <#9430 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXFVC5YKEREYD3W7V7YBZOLXIC5DJANCNFSM6AAAAAAYGIPRPM> . You are receiving this because you commented.Message ID: ***@***.***>

[xyhp915]

Just to clarify I'm asking if it's a breaking change, not when we should do work to make it backwards compatible. Is it a breaking change? If so, we should mention it in the PR title as users have complained when we don't communicate breaking changes. If it's a minor we can call it "minor behavior change" like we did in #9291
…
On Fri, 26 May 2023 at 10:31, Charlie @.> wrote: @.* commented on this pull request. ------------------------------ In src/main/frontend/modules/shortcut/config.cljs <#9430 (comment)>: > @@ -113,19 +113,19 @@ :whiteboard/zoom-out {:binding "shift+dash" :fn #(.zoomOut (.-api ^js (state/active-tldraw-app)) false)} - :whiteboard/zoom-in {:binding "shift+=" + :whiteboard/zoom-in {:binding "shift+equals" I think it's best to support them in later refactorings. — Reply to this email directly, view it on GitHub <#9430 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXFVC5YKEREYD3W7V7YBZOLXIC5DJANCNFSM6AAAAAAYGIPRPM . You are receiving this because you commented.Message ID: @.***>

Gotcha! It's not a breaking change. Also, it's just a fix :)

[xyhp915]

@andelf @cnrpman Asking for code reviewers :)

[cnrpman]

LGTM