Skip to content

Commit

Permalink
Visualizations fixes follow-up (#9252)
Browse files Browse the repository at this point in the history
Addressing review suggestions from #9130

- Removing `N` binding
- Removing duplicated `Toggle fullscreen vis` binding

A few important differences from the suggested implementation:
1. There is no easy way to implement `nextType` on GraphEditor – we simply don’t have the required API
2. The keydown handler in `GraphVisualization` must be defined on window level still, otherwise it won’t get keydown events unless visualization is focused, and thus `nextType` won’t work because of (1)

No visual changes to the IDE.
  • Loading branch information
vitvakatu authored Mar 8, 2024
1 parent ee2dc57 commit df72b38
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 17 deletions.
2 changes: 0 additions & 2 deletions app/gui2/src/bindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ export const graphBindings = defineKeybinds('graph-editor', {
redo: ['Mod+Y', 'Mod+Shift+Z'],
dragScene: ['PointerAux', 'Mod+PointerMain'],
openComponentBrowser: ['Enter'],
newNode: ['N'],
toggleVisualization: ['Space'],
toggleVisualizationFullscreen: ['Shift+Space'],
deleteSelected: ['OsDelete'],
zoomToSelected: ['Mod+Shift+A'],
selectAll: ['Mod+A'],
Expand Down
14 changes: 0 additions & 14 deletions app/gui2/src/components/GraphEditor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,6 @@ const graphBindingsHandler = graphBindings.handler({
showComponentBrowser()
}
},
newNode() {
if (keyboardBusy()) return false
if (graphNavigator.sceneMousePos != null) {
graphStore.createNode(graphNavigator.sceneMousePos, 'hello "world"! 123 + x')
}
},
deleteSelected() {
graphStore.transact(() => {
graphStore.deleteNodes([...nodeSelection.selected])
Expand Down Expand Up @@ -267,14 +261,6 @@ const graphBindingsHandler = graphBindings.handler({
}
})
},
toggleVisualizationFullscreen() {
if (nodeSelection.selected.size !== 1) return
graphStore.transact(() => {
const selected = set.first(nodeSelection.selected)
const isFullscreen = graphStore.db.nodeIdToNode.get(selected)?.vis?.fullscreen
graphStore.setNodeVisualization(selected, { visible: true, fullscreen: !isFullscreen })
})
},
copyNode() {
if (keyboardBusy()) return false
copyNodeContent()
Expand Down
2 changes: 1 addition & 1 deletion app/gui2/src/components/GraphEditor/GraphVisualization.vue
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ const keydownHandler = visualizationBindings.handler({
},
})
useEvent(window, 'keydown', (event) => keydownHandler(event))
useEvent(window, 'keydown', keydownHandler)
watch(
() => props.isFullscreen,
Expand Down

0 comments on commit df72b38

Please sign in to comment.