Initial keyboard shortcuts configuration implementation #496

Merged
franknoirot merged 11 commits from keyboard-shortcuts into main 2021-09-10 00:12:59 +02:00
2 changed files with 3 additions and 15 deletions
Showing only changes of commit 70cbe9d11e - Show all commits

View File

@@ -18,22 +18,8 @@ const AllShortcutsModal = () => {
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
const classes = useStyles()
const [open, setOpen] = useState(false)
useHotkeys(SHORTCUT, () => setOpen(!open), [open])
const { state, thunkDispatch } = useIdeContext()
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
return (<>
<div className="hidden">
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
<Menu>
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
{ editorMenuConfig.filter(menu => menu.items.length).map(menu =>
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
menu.items.map(item => (
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
<item.component
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
state={state}
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
thunkDispatch={thunkDispatch}
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
config={item}
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
key={menu.label +"-"+ item.label } />
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
))
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
)}
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
</Menu>
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
</div>
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
<Dialog open={open} onClose={() => setOpen(false)} className={classes.root}>
<div className="bg-ch-gray-700 font-fira-sans max-w-7xl rounded shadow-lg text-ch-gray-300 p-4">
<h2 className="text-2xl mb-4">All Shortcuts</h2>
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
Irev-Dev commented 2021-09-08 10:00:35 +02:00 (Migrated from github.com)
Review

Should this also have a button in the menu, how will users learn how to get this modal up?

Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused.

Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.

Should this also have a button in the menu, how will users learn how to get this modal up? Why did you choose this combo, is this a pattern elsewhere? I'm concerned that is the same hotkey as commenting a line in the editor, since the editor is currently swallowing this hotkey so it still works in the editor but seems weird having two different behaviour depending on where you're focused. Maybe it's a mac only thing, but similar to the above cmd alt R only resets the viewer when I'm not focused on the edit, because its already a hotkey in the editor. Seems like we're going to be competing for hotkeys from the monaco editor.
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:38:07 +02:00 (Migrated from github.com)
Review

Ah crap this is all good consideration @Irev-Dev.

Here are my recommendations for these 2:

  • All Shortcuts:command+/ 👉🏻 ctrl+shift+?. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View?
  • Reset Layout: command+alt+R 👉🏻 ctrl+shift+R. It appears that ctrl+shift is a fairly reliable key combination.

Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?

Ah crap this is all good consideration @Irev-Dev. Here are my recommendations for these 2: - **All Shortcuts**:`command+/` 👉🏻 `ctrl+shift+?`. This is what Figma's shortcut to show shortcuts is. I agree this should have a button in the menu. Would it go under View? - **Reset Layout**: `command+alt+R` 👉🏻 `ctrl+shift+R`. It appears that `ctrl+shift` is a fairly reliable key combination. Separately, should we have a part of this component or its tests have a list of the Monaco shortcuts to verify any new ones aren't stepping on them?
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
franknoirot commented 2021-09-08 15:53:44 +02:00 (Migrated from github.com)
Review

One thing that's weird to me is that HotKeys treats Ctrl and Cmd as the Windows/Mac alternatives, but it's really Cmd and Win keys that are analogous, usually called the OS key within the browser, and Ctrl is a key that both OSes have.

One thing that's weird to me is that HotKeys treats `Ctrl` and `Cmd` as the Windows/Mac alternatives, but it's really `Cmd` and `Win` keys that are analogous, usually called the OS key within the browser, and `Ctrl` is a key that both OSes have.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
Irev-Dev commented 2021-09-09 10:54:34 +02:00 (Migrated from github.com)
Review
  • ctrl shift ? I think make sense for the modal :)
  • test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves.
  • I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
- ctrl shift ? I think make sense for the modal :) - test might be good, but not sure if there is a way it pull hotkeys out of monaco without hardcoding them ourselves. - I think the library does that for practical reasons since ctrl is the most common key used for hotkey for win and cmd is the most common for mac, for example cmd c, cmd v is copy paste.
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
franknoirot commented 2021-09-09 16:23:00 +02:00 (Migrated from github.com)
Review

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find this GH issue comment about it. There doesn't appear to be like KeybindingsRegistry.getAll() method or anything like that.

The Monaca online editor is built on top of Monaco and has this hardcoded list of shortcuts that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And this issue has been raised in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really.

Want me to open an issue for the tests? I feel like we can do that a bit later, right?

Yeah doesn't appear to be a great way to pull them out programmatically, only was able to find [this GH issue comment about it](https://github.com/microsoft/monaco-editor/issues/823#issuecomment-470754000). There doesn't appear to be like `KeybindingsRegistry.getAll()` method or anything like that. The Monaca online editor is built on top of Monaco and has this hardcoded [list of shortcuts](https://docs.monaca.io/en/products_guide/monaca_ide/editor/) that we could pull from for a hardcoded solution. I think they'll be pretty stable so that might not be too bad? And [this issue has been raised](https://github.com/microsoft/monaco-editor/issues/102) in Monaco for an easy API for swapping out keyboard shortcuts, I couldn't find a clean holistic API for keybindings at all really. Want me to open an issue for the tests? I feel like we can do that a bit later, right?
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
Irev-Dev commented 2021-09-09 21:20:29 +02:00 (Migrated from github.com)
Review

Yeah sure, sounds good.

Yeah sure, sounds good.
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this
franknoirot commented 2021-09-10 00:12:40 +02:00 (Migrated from github.com)
Review

Added issue #499 to track this

Added issue #499 to track this

View File

@@ -35,12 +35,14 @@ export function Dropdown({
return (
<div className="relative">
<Menu>
{({ open }) => (<>
<Menu.Button className={"text-gray-100" + (disabled ? " text-gray-400 cursor-not-allowed" : "")} disabled={disabled}>{label}</Menu.Button>
{ children &&
<Menu.Items className="absolute flex flex-col mt-4 bg-ch-gray-760 rounded text-gray-100 overflow-hidden whitespace-nowrap border border-ch-gray-700">
<Menu.Items static className={(open ? "" : "hidden ") + "absolute flex flex-col mt-4 bg-ch-gray-760 rounded text-gray-100 overflow-hidden whitespace-nowrap border border-ch-gray-700"}>
{children}
</Menu.Items>
}
</>)}
</Menu>
</div>
)