Eng 1552 enable ability to change discourse graph specific keyboard#952
Open
Eng 1552 enable ability to change discourse graph specific keyboard#952
Conversation
Allow users to customize discourse node shortcuts in the canvas via a new "Canvas shortcuts" tab in personal settings.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Collaborator
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
📝 WalkthroughWalkthroughThis PR introduces customizable keyboard shortcuts for canvas nodes. A new personal settings field stores node-specific shortcut overrides, a settings UI component allows configuration, the schema validates the setting structure, and the canvas UI layer reads and applies these overrides when rendering node tools. Changes
Sequence DiagramsequenceDiagram
actor User
participant Settings as CanvasShortcutSettings UI
participant PersonalSettings as Personal Settings Store
participant Canvas as Canvas Component
participant UIOverrides as uiOverrides Logic
User->>Settings: Opens Canvas shortcuts tab
Settings->>PersonalSettings: Reads current shortcuts via getPersonalSetting()
PersonalSettings-->>Settings: Returns shortcuts map
Settings->>User: Displays shortcut fields for each node type
User->>Settings: Customizes shortcut for a node
Settings->>PersonalSettings: Saves updated shortcut mapping
PersonalSettings-->>Settings: Confirms save
Note over Canvas,UIOverrides: Next Canvas Load
Canvas->>UIOverrides: createUiOverrides() initializes
UIOverrides->>PersonalSettings: Reads Canvas node shortcuts setting
PersonalSettings-->>UIOverrides: Returns shortcut overrides map
UIOverrides->>UIOverrides: For each node tool, override kbd with custom shortcut or default
UIOverrides-->>Canvas: Returns configured tools with applied shortcuts
Canvas->>User: Renders canvas with custom node shortcuts active
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/roam/src/components/settings/CanvasShortcutSettings.tsx (1)
8-8: Consider using UPPERCASE for the constant name.Per coding guidelines, constants should use UPPERCASE naming convention.
💡 Suggested change
-const CANVAS_NODE_SHORTCUTS_KEY = "Canvas node shortcuts"; +const CANVAS_NODE_SHORTCUTS_KEY = "Canvas node shortcuts"; // Already uppercase, but could be more explicitThe constant name
CANVAS_NODE_SHORTCUTS_KEYalready uses uppercase, so this is compliant. No change needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/roam/src/components/settings/CanvasShortcutSettings.tsx` at line 8, The reviewer suggested using UPPERCASE for the constant name, but the constant CANVAS_NODE_SHORTCUTS_KEY in CanvasShortcutSettings.tsx is already UPPERCASE; no code change is needed—leave CANVAS_NODE_SHORTCUTS_KEY as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/roam/src/components/settings/CanvasShortcutSettings.tsx`:
- Line 8: The reviewer suggested using UPPERCASE for the constant name, but the
constant CANVAS_NODE_SHORTCUTS_KEY in CanvasShortcutSettings.tsx is already
UPPERCASE; no code change is needed—leave CANVAS_NODE_SHORTCUTS_KEY as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: eb5a2020-f01b-486a-97e1-b7bfee584cb1
📒 Files selected for processing (5)
apps/roam/src/components/canvas/uiOverrides.tsxapps/roam/src/components/settings/CanvasShortcutSettings.tsxapps/roam/src/components/settings/Settings.tsxapps/roam/src/components/settings/utils/zodSchema.example.tsapps/roam/src/components/settings/utils/zodSchema.ts
…empty)
Per review feedback: each node now has an explicit "use custom shortcut"
toggle. Unchecked falls back to the global default; checked with a value
overrides; checked with empty value disables the shortcut entirely.
- Schema: `Record<nodeType, { value, enabled }>` replaces flat string map
- Read side in createUiOverrides keys off `enabled` flag, not value presence
- Each row uses PersonalFlagPanel + PersonalTextPanel; flag controls input's
disabled state, uncheck clears stored value via remount
- BaseTextPanel: added `disabled?: boolean` and made Description conditional
on a non-empty description (no-op for existing callers)
- Exported `CanvasNodeShortcuts` type from zodSchema; uiOverrides imports it
…k props
Settings tab opened slowly (~5s) because each row called getPersonalSetting
twice — once on mount and again every render — and each call ran a Datalog
query plus a full PersonalSettingsSchema.parse(). createUiOverrides hit the
same accessor too.
Switched to the same pattern other personal settings use (KeyboardShortcutInput,
HomePersonalSettings flags): read from the Roam extensionAPI, dual-write to
extensionAPI + block props on change. extensionAPI reads are sync and
in-memory.
- New constant CANVAS_NODE_SHORTCUTS_KEY in data/userSettings.ts
- CanvasShortcutSettings: parent reads the record once, holds it in state,
passes initial values down; row notifies parent on change, parent writes to
extensionAPI; PersonalFlagPanel/PersonalTextPanel keep their internal
block-prop writes so block props stay in sync
- uiOverrides: getSetting(CANVAS_NODE_SHORTCUTS_KEY, {}) instead of
getPersonalSetting(["Canvas node shortcuts"])
mdroidian
requested changes
Apr 22, 2026
Member
mdroidian
left a comment
mdroidian
left a comment
There was a problem hiding this comment.
Let's change the UI to something like this:
- Single tab: Shortcuts
- Label: {nodeLabel} node
- Single row for each
- Remove repeated description from tooltips, add to top of page
Ideally we reduce the size of the input box, but that is optional, depending on the component used.
| return ( | ||
| <div className="flex flex-col gap-1"> | ||
| <PersonalFlagPanel | ||
| title={`Use custom ${nodeText} shortcut`} |
Member
There was a problem hiding this comment.
Let's DRY this.
Right now "use custom ... shortcut" is repeated multiple times.
Instead, let's add a single tab
"Shortcuts"
And change the title of the panels to just "${nodeText} node"
| <div className="flex flex-col gap-1"> | ||
| <PersonalFlagPanel | ||
| title={`Use custom ${nodeText} shortcut`} | ||
| description={`Override the canvas keyboard shortcut. Default: ${defaultShortcut || "none"}. Changes take effect next time a canvas is opened.`} |
Member
There was a problem hiding this comment.
Let's move "Override the canvas keyboard shortcut." and "Changes take effect next time a canvas is opened." to a the top of the page, inside a single tab.
| onChange={handleEnabledChange} | ||
| /> | ||
| <div className="ml-6 max-w-xs"> | ||
| <PersonalTextPanel |
Member
There was a problem hiding this comment.
This should be restricted to a single key, but right now it can take in any arbitrary text.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://www.loom.com/share/c79b2d497a60482a8536dd9452b37868
Summary by CodeRabbit
Release Notes