-
Notifications
You must be signed in to change notification settings - Fork 321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Store additional metainformation for defineKeybinds #9427
Conversation
To display human-readable shortcuts in UI and use them in tests
Dashboard’s div was covering it
@@ -217,7 +217,7 @@ | |||
--side-panel-label-width: 8rem; | |||
--side-panel-description-padding-y: 0.25rem; | |||
|
|||
--extended-editor-menu-size: 2.5rem; | |||
--extended-editor-menu-size: 48px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for reference, 1rem = 16px. but i suppose this is more robust as the menu will always be a fixed pixel size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I guess we want to use pixels here, as we know exact size of the button and exact pixel paddings around it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approving (minimal) dashboard changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please merge is ASAP, as it contains fix for important issue #9445
// TODO[ao]: Skipping test, as they often fail in CI | ||
// (for example https://github.com/enso-org/enso/actions/runs/8102076908/job/22163122663) | ||
test.each` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the overlooked TODO.
@@ -52,44 +59,44 @@ const areas: Record<string, Rect> = { | |||
// TODO[ao]: Skipping test, as they often fail in CI | |||
// (for example https://github.com/enso-org/enso/actions/runs/8102076908/job/22163122663) | |||
test.each` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here
app/gui2/src/util/shortcuts.ts
Outdated
@@ -265,7 +299,7 @@ export const DefaultHandler = Symbol('default handler') | |||
* @param bindings is an object defining actions and their key bindings. Each property name is an | |||
* action name, and value is list of default key bindings. See "Keybinds should be parsed | |||
* correctly" test for examples of valid strings. | |||
* @returns an object with defined `handler` function. | |||
* @returns an object with defined `handler` function and `bindings`, containing information about assigned bindings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Over 100 characters. Alas, prettier does not handle docstrings
Pull Request Description
Closes #9411
Based on #9365
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build
.