Delete Object Keyboard Shortcut #388
Conversation
|
|
There was a problem hiding this comment.
Neat! Thanks.
A small issue I spotted, when selecting an edge the title of the dialog is kind-of wrong, because we cannot just delete an edge. So we should probably find the parent object of the edge and delete that parent instead.
Screencast.from.2024-07-29.09-28-47.mp4
packages/base/src/keybindings.json
Outdated
| [ | ||
| { | ||
| "command": "jupytercad:removeObject", | ||
| "keys": ["Accel X"], |
There was a problem hiding this comment.
I feel like "Accel X" should be the keybinding for "cut", though we don't have any copy/paste logic available so I'm fine keeping this keybinding for now if you like 👍🏽
Though should we also add "Delete" there too in the list?
There was a problem hiding this comment.
Thanks, that makes much sense as xref:QuantStack/jupytergis#68 also uses the [Delete] key for the same.
Also it sounds nice to have a feature request open for cut/copy/paste logic. I can open one if it's fine.
There was a problem hiding this comment.
I can open one if it's fine
Sounds good!
| { | ||
| "command": "jupytercad:removeObject", | ||
| "keys": ["Accel X"], | ||
| "selector": ".jp-jupytercad-panel" |
There was a problem hiding this comment.
This results in the key binding to not be triggered if it's the left panel (object list) that has focus.
I wonder if we should add the keybinding to the #main application instead. Though I tested locally and that does not seem to work, I suppose it's because the command depends on the WidgetTracker<JupyterCadWidget> tracker, and that one only tracks the 3D view if I'm correct.
Let's not consider this a blocker for merging the PR, but let's keep track of this in an issue after merging.
There was a problem hiding this comment.
I see, i can try adding the valid className for left panel too and see if the union of jupytercad-panel & leftpanel works.
There was a problem hiding this comment.
I thought it was easy.
But it was not as simple as I thought 😅, there seem to be a complexity with the Document Object Model that all the components are not naturally focusable on click, which was not letting me to have a specific component as a selector.
But at last it was done by defining tabIndex={0} & one can now delete object with key binding while being on ObjectTree too.
Thanks for merging this @martinRenou, And sorry for the delay I was still looking into the edge issue. I'll open a PR upstream as soon as i manage to fix it and if not I'll definitely open an issue to keep track of it. |
|
Great! Thanks for working on it |
Should fix #46
Adds a
Ctrl+Xkeybinding to delete object.