Initial keyboard shortcuts configuration implementation #496

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

View File

@@ -1,5 +1,5 @@
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
import { Menu } from '@headlessui/react'
import { useEffect, useState } from 'react'
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
import { createContext, useContext, useEffect, useState } from 'react'
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
import { useHotkeys } from 'react-hotkeys-hook'
import { makeStyles } from '@material-ui/core/styles'
import Dialog from '@material-ui/core/Dialog'
@@ -14,19 +14,37 @@ const useStyles = makeStyles({
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
},
})
interface ShortcutsModalContextType {
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
open: boolean
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
toggleOpen: () => any
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
export const ShortcutsModalContext = createContext<ShortcutsModalContextType>({
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
open: false,
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
toggleOpen: () => {},
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 09:53:33 +02:00 (Migrated from github.com)
Review

Seems to be a weird artifact on the rounded corner, not sure if worth fixing?
image

Seems to be a weird artifact on the rounded corner, not sure if worth fixing? ![image](https://user-images.githubusercontent.com/29681384/132469191-d50fe9aa-dfed-43ca-8cbc-b83a198182e3.png)
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:27:02 +02:00 (Migrated from github.com)
Review

Oh yeah lemme check it out, I bet it's a difference between the Modal and the inner content, and maybe I rounded the inner stuff as well and they're basically overlapping.

Oh yeah lemme check it out, I bet it's a difference between the Modal and the inner content, and maybe I rounded the inner stuff as well and they're basically overlapping.
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
export function useShortcutsModalContext() {
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 useContext(ShortcutsModalContext)
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
const AllShortcutsModal = () => {
const classes = useStyles()
const [open, setOpen] = useState(false)
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
useHotkeys(SHORTCUT, () => setOpen(!open), [open])
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
const { open, toggleOpen } = useShortcutsModalContext()
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 (
<>
<Dialog
open={open}
onClose={() => setOpen(false)}
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
className={classes.root}
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
onClose={() => toggleOpen()}
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
className={classes.root + ' bg-transparent'}
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
PaperProps={{
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
style: {
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
backgroundColor: 'transparent',
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
>
<div className="bg-ch-gray-700 font-fira-sans max-w-7xl rounded shadow-lg text-ch-gray-300 p-4">
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 className="bg-ch-gray-700 font-fira-sans shadow-lg text-ch-gray-300 p-4">
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
<h2 className="text-2xl mb-4">All Shortcuts</h2>
{editorMenuConfig
franknoirot commented 2021-09-08 17:36:58 +02:00 (Migrated from github.com)
Review

Yeah overwriting MaterialUI's default styles is not intuitive

Yeah overwriting MaterialUI's default styles is not intuitive
Irev-Dev commented 2021-09-09 11:05:28 +02:00 (Migrated from github.com)
Review

Mmm, we'll move towards headlessui where possible in future. Very tailwind friendly, and friendly to customising styles in general too.

Mmm, we'll move towards headlessui where possible in future. Very tailwind friendly, and friendly to customising styles in general too.
.filter((menu) => menu.items.length)
@@ -35,7 +53,7 @@ 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
<h3 className="text-xl border-b-2 pb-2 mb-2">{menu.label}</h3>
{menu.items.map((item) => (
<div
className="flex gap-8 justify-between"
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
className="flex gap-16 justify-between"
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={'allshortcuts-' + menu.name + '-' + item.label}
>
<p>{item.label}</p>
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

@@ -3,6 +3,7 @@ import { useRender } from 'src/components/IdeWrapper/useRender'
Irev-Dev commented 2021-09-08 10:35:59 +02:00 (Migrated from github.com)
Review

if you make this:

const fileMenuConfig: EditorMenuConfig = {

Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since component doesn't exist on EditorMenuItemConfig.

if you make this: ```suggestion const fileMenuConfig: EditorMenuConfig = { ``` Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since `component` doesn't exist on `EditorMenuItemConfig`.
Irev-Dev commented 2021-09-08 10:35:59 +02:00 (Migrated from github.com)
Review

if you make this:

const fileMenuConfig: EditorMenuConfig = {

Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since component doesn't exist on EditorMenuItemConfig.

if you make this: ```suggestion const fileMenuConfig: EditorMenuConfig = { ``` Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since `component` doesn't exist on `EditorMenuItemConfig`.
Irev-Dev commented 2021-09-08 10:38:23 +02:00 (Migrated from github.com)
Review

I think callback is not needed now and component is?

I think callback is not needed now and component is?
Irev-Dev commented 2021-09-08 10:38:23 +02:00 (Migrated from github.com)
Review

I think callback is not needed now and component is?

I think callback is not needed now and component is?
franknoirot commented 2021-09-08 16:00:31 +02:00 (Migrated from github.com)
Review

Yup it yelled at me now!

Yup it yelled at me now!
franknoirot commented 2021-09-08 16:00:31 +02:00 (Migrated from github.com)
Review

Yup it yelled at me now!

Yup it yelled at me now!
import { makeStlDownloadHandler, PullTitleFromFirstLine } from './helpers'
import { useSaveCode } from 'src/components/IdeWrapper/useSaveCode'
franknoirot commented 2021-09-08 16:39:40 +02:00 (Migrated from github.com)
Review

This won't work currently because I backed out of writing it, but @Irev-Dev should I use a React Context to make the AllShortcutsModal able to be opened from outside of the component? I could make its own little context to provide around the IdeWrapper component, or the page component itself. I might try that and see if you like it.

This won't work currently because I backed out of writing it, but @Irev-Dev should I use a React Context to make the `AllShortcutsModal` able to be opened from outside of the component? I could make its own little context to provide around the `IdeWrapper` component, or the page component itself. I might try that and see if you like it.
franknoirot commented 2021-09-08 17:39:09 +02:00 (Migrated from github.com)
Review

Tried this approach below.

Tried this approach below.
import { DropdownItem } from './Dropdowns'
import { useShortcutsModalContext } from './AllShortcutsModal'
Irev-Dev commented 2021-09-08 10:35:59 +02:00 (Migrated from github.com)
Review

if you make this:

const fileMenuConfig: EditorMenuConfig = {

Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since component doesn't exist on EditorMenuItemConfig.

if you make this: ```suggestion const fileMenuConfig: EditorMenuConfig = { ``` Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since `component` doesn't exist on `EditorMenuItemConfig`.
Irev-Dev commented 2021-09-08 10:38:23 +02:00 (Migrated from github.com)
Review

I think callback is not needed now and component is?

I think callback is not needed now and component is?
franknoirot commented 2021-09-08 16:00:31 +02:00 (Migrated from github.com)
Review

Yup it yelled at me now!

Yup it yelled at me now!
export function cmdOrCtrl() {
return /(Mac|iPhone|iPod|iPad)/i.test(navigator.platform) ? '⌘' : 'Ctrl'
@@ -83,8 +84,8 @@ const viewMenuConfig = {
Irev-Dev commented 2021-09-08 10:35:59 +02:00 (Migrated from github.com)
Review

if you make this:

const fileMenuConfig: EditorMenuConfig = {

Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since component doesn't exist on EditorMenuItemConfig.

if you make this: ```suggestion const fileMenuConfig: EditorMenuConfig = { ``` Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since `component` doesn't exist on `EditorMenuItemConfig`.
Irev-Dev commented 2021-09-08 10:35:59 +02:00 (Migrated from github.com)
Review

if you make this:

const fileMenuConfig: EditorMenuConfig = {

Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since component doesn't exist on EditorMenuItemConfig.

if you make this: ```suggestion const fileMenuConfig: EditorMenuConfig = { ``` Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since `component` doesn't exist on `EditorMenuItemConfig`.
Irev-Dev commented 2021-09-08 10:38:23 +02:00 (Migrated from github.com)
Review

I think callback is not needed now and component is?

I think callback is not needed now and component is?
Irev-Dev commented 2021-09-08 10:38:23 +02:00 (Migrated from github.com)
Review

I think callback is not needed now and component is?

I think callback is not needed now and component is?
franknoirot commented 2021-09-08 16:00:31 +02:00 (Migrated from github.com)
Review

Yup it yelled at me now!

Yup it yelled at me now!
franknoirot commented 2021-09-08 16:00:31 +02:00 (Migrated from github.com)
Review

Yup it yelled at me now!

Yup it yelled at me now!
shortcutLabel: 'Ctrl Shift /',
component: (props) => {
const { config } = props
const [open, setOpen] = useShortcutModalContext()
Irev-Dev commented 2021-09-08 10:35:59 +02:00 (Migrated from github.com)
Review

if you make this:

const fileMenuConfig: EditorMenuConfig = {

Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since component doesn't exist on EditorMenuItemConfig.

if you make this: ```suggestion const fileMenuConfig: EditorMenuConfig = { ``` Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since `component` doesn't exist on `EditorMenuItemConfig`.
Irev-Dev commented 2021-09-08 10:38:23 +02:00 (Migrated from github.com)
Review

I think callback is not needed now and component is?

I think callback is not needed now and component is?
franknoirot commented 2021-09-08 16:00:31 +02:00 (Migrated from github.com)
Review

Yup it yelled at me now!

Yup it yelled at me now!
config.callback = () => setOpen(true)
Irev-Dev commented 2021-09-08 10:35:59 +02:00 (Migrated from github.com)
Review

if you make this:

const fileMenuConfig: EditorMenuConfig = {

Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since component doesn't exist on EditorMenuItemConfig.

if you make this: ```suggestion const fileMenuConfig: EditorMenuConfig = { ``` Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since `component` doesn't exist on `EditorMenuItemConfig`.
Irev-Dev commented 2021-09-08 10:38:23 +02:00 (Migrated from github.com)
Review

I think callback is not needed now and component is?

I think callback is not needed now and component is?
franknoirot commented 2021-09-08 16:00:31 +02:00 (Migrated from github.com)
Review

Yup it yelled at me now!

Yup it yelled at me now!
const { toggleOpen } = useShortcutsModalContext()
Irev-Dev commented 2021-09-08 10:35:59 +02:00 (Migrated from github.com)
Review

if you make this:

const fileMenuConfig: EditorMenuConfig = {

Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since component doesn't exist on EditorMenuItemConfig.

if you make this: ```suggestion const fileMenuConfig: EditorMenuConfig = { ``` Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since `component` doesn't exist on `EditorMenuItemConfig`.
Irev-Dev commented 2021-09-08 10:38:23 +02:00 (Migrated from github.com)
Review

I think callback is not needed now and component is?

I think callback is not needed now and component is?
franknoirot commented 2021-09-08 16:00:31 +02:00 (Migrated from github.com)
Review

Yup it yelled at me now!

Yup it yelled at me now!
config.callback = toggleOpen
Irev-Dev commented 2021-09-08 10:35:59 +02:00 (Migrated from github.com)
Review

if you make this:

const fileMenuConfig: EditorMenuConfig = {

Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since component doesn't exist on EditorMenuItemConfig.

if you make this: ```suggestion const fileMenuConfig: EditorMenuConfig = { ``` Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since `component` doesn't exist on `EditorMenuItemConfig`.
Irev-Dev commented 2021-09-08 10:38:23 +02:00 (Migrated from github.com)
Review

I think callback is not needed now and component is?

I think callback is not needed now and component is?
franknoirot commented 2021-09-08 16:00:31 +02:00 (Migrated from github.com)
Review

Yup it yelled at me now!

Yup it yelled at me now!
return <DropdownItem {...props} />
},
},
Irev-Dev commented 2021-09-08 10:35:59 +02:00 (Migrated from github.com)
Review

if you make this:

const fileMenuConfig: EditorMenuConfig = {

Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since component doesn't exist on EditorMenuItemConfig.

if you make this: ```suggestion const fileMenuConfig: EditorMenuConfig = { ``` Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since `component` doesn't exist on `EditorMenuItemConfig`.
Irev-Dev commented 2021-09-08 10:35:59 +02:00 (Migrated from github.com)
Review

if you make this:

const fileMenuConfig: EditorMenuConfig = {

Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since component doesn't exist on EditorMenuItemConfig.

if you make this: ```suggestion const fileMenuConfig: EditorMenuConfig = { ``` Thdn the linter (assuming you've got ts setup with your editor) should tell you if your obeying your own types for this menu, and I think you're not since `component` doesn't exist on `EditorMenuItemConfig`.
Irev-Dev commented 2021-09-08 10:38:23 +02:00 (Migrated from github.com)
Review

I think callback is not needed now and component is?

I think callback is not needed now and component is?
Irev-Dev commented 2021-09-08 10:38:23 +02:00 (Migrated from github.com)
Review

I think callback is not needed now and component is?

I think callback is not needed now and component is?
franknoirot commented 2021-09-08 16:00:31 +02:00 (Migrated from github.com)
Review

Yup it yelled at me now!

Yup it yelled at me now!
franknoirot commented 2021-09-08 16:00:31 +02:00 (Migrated from github.com)
Review

Yup it yelled at me now!

Yup it yelled at me now!

View File

@@ -8,6 +8,7 @@ import Svg from 'src/components/Svg/Svg'
import { useIdeInit } from 'src/components/EncodedUrl/helpers'
import { useIdeContext } from 'src/helpers/hooks/useIdeContext'
import { useSaveCode } from 'src/components/IdeWrapper/useSaveCode'
import { ShortcutsModalContext } from 'src/components/EditorMenu/AllShortcutsModal'
interface Props {
cadPackage: string
@@ -22,18 +23,25 @@ const IdeWrapper = ({ cadPackage }: Props) => {
saveCode({ code: state.code })
}
useIdeInit(cadPackage, project?.code || state?.code)
const [shortcutModalOpen, setShortcutModalOpen] = useState(false)
const shortcutModalContextValues = {
open: shortcutModalOpen,
toggleOpen: () => setShortcutModalOpen(!shortcutModalOpen),
}
return (
<div className="h-full flex">
<div className="w-16 bg-ch-gray-700 flex-shrink-0">
<IdeSideBar />
</div>
<div className="h-full flex flex-grow flex-col">
<nav className="flex">
<IdeHeader handleRender={onRender} />
</nav>
<IdeContainer />
</div>
<ShortcutsModalContext.Provider value={shortcutModalContextValues}>
<div className="w-16 bg-ch-gray-700 flex-shrink-0">
<IdeSideBar />
</div>
<div className="h-full flex flex-grow flex-col">
<nav className="flex">
<IdeHeader handleRender={onRender} />
</nav>
<IdeContainer />
</div>
</ShortcutsModalContext.Provider>
</div>
)
}