-
Notifications
You must be signed in to change notification settings - Fork 786
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
fix command palette key #4890
fix command palette key #4890
Conversation
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.
Thanks Will for taking another look at this!
I don't think this needs a 'fixed' entry in the CHANGELOG, as the command palette binding change wasn't yet released?
However as mentioned in #4857 (comment), there's currently nothing in the CHANGELOG for this binding change. which might be breaking change if apps have their own bindings for Ctrl+p?
|
||
class NewPaletteBindingApp(App): | ||
COMMAND_PALETTE_BINDING = "ctrl+backslash" | ||
COMMAND_PALETTE_DISPLAY = "ctrl+\\" |
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.
Perhaps I've misunderstood this new COMMAND_PALETTE_DISPLAY
variable , but why does the snapshot show ^\
rather than ctrl+\
?
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.
There is some work in #4876 that will fix that.
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.
It might be worth adding that work in this PR if possible, as currently the snapshot is not really as expected?
CHANGELOG.md
Outdated
@@ -16,17 +16,20 @@ and this project adheres to [Semantic Versioning](http://semver.org/). | |||
- Added "Show keys" option to system commands to show a summary of key bindings. https://github.com/Textualize/textual/pull/4876 | |||
- Added "split" CSS style, currently undocumented, and may change. https://github.com/Textualize/textual/pull/4876 | |||
- Added `Region.get_spacing_between` https://github.com/Textualize/textual/pull/4876 | |||
- Added `App.COMMAND_PALETTE_KEY` to change default command palette key binding |
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.
Remember to link this PR
Started out as a fix for the command palette key binding. Turned in to a refactor and an alternative centralised way of defining how keys are displayed in footer and key panel.
Fixes #4887
Fixes #4901