-
Notifications
You must be signed in to change notification settings - Fork 11
feat: enhance command palette integration for SmartBlocks #139
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
Conversation
- Added support for a new <%CMD%> tag to opt specific workflows into the Roam command palette. - Updated documentation to reflect the new command palette opt-in feature and usage instructions. - Implemented functionality to refresh command palette workflows after changes to the <%CMD%> tag. - Refactored command palette command management in the code for better clarity and performance.
|
Caution Review failedThe pull request is closed. WalkthroughWorkflows can opt into Roam's Command Palette by tagging titles with <%CMD%> and enabling a new Command Palette Opt‑In setting. Implementation adds command lifecycle helpers, a refresh command, regex-based detection/cleaning, docs updates, and a package version bump. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SettingsUI as Settings UI
participant Sync as syncCommandPaletteCommands()
participant RoamCP as Roam Command Palette
User->>SettingsUI: Toggle "Command Palette Opt‑In"
SettingsUI->>Sync: Trigger syncCommandPaletteCommands()
Sync->>Sync: removeCommandPaletteCommands()
Sync->>RoamCP: Clear existing commands
alt commandPaletteEnabled
Sync->>Sync: addCommandPaletteCommands() (filter by opt‑in & eligibility)
Sync->>RoamCP: Register tagged workflows
end
RoamCP->>User: Updated command palette
sequenceDiagram
actor User
participant Palette as Command Palette
participant RefreshCmd as "Refresh SmartBlocks" Command
participant Sync as syncCommandPaletteCommands()
participant Toast as Toast
User->>Palette: Invoke "Refresh SmartBlocks Command Palette"
Palette->>RefreshCmd: Execute
RefreshCmd->>Sync: Call syncCommandPaletteCommands()
Sync->>Palette: Re-synchronize commands
RefreshCmd->>Toast: Show confirmation
Toast->>User: Display success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/index.ts (3)
131-174: De‑duplicate labels and record them for reliable removal.Multiple workflows with the same cleaned name will collide on a single label. Also, add each label to the tracking set you introduced so
removeCommandPaletteCommandscan clean them up.Apply:
const addCommandPaletteCommands = () => { const eligibleWorkflows = getVisibleCustomWorkflows().filter((wf) => commandPaletteOptIn ? wf.commandPaletteEligible : true ); - getCleanCustomWorkflows(eligibleWorkflows).forEach((wf) => { - window.roamAlphaAPI.ui.commandPalette.addCommand({ - label: `Trigger SmartBlock: ${wf.name}`, + const labelsThisSync = new Set<string>(); + getCleanCustomWorkflows(eligibleWorkflows).forEach((wf) => { + const base = `Trigger SmartBlock: ${wf.name}`; + let label = base; + let i = 2; + while (labelsThisSync.has(label)) label = `${base} (${i++})`; + window.roamAlphaAPI.ui.commandPalette.addCommand({ + label, callback: () => { const targetUid = window.roamAlphaAPI.ui.getFocusedBlock()?.["block-uid"]; // Because the command palette does a blur event on close, // we want a slight delay so that we could keep focus window.setTimeout(() => { if (targetUid) { sbBomb({ srcUid: wf.uid, target: { uid: targetUid, isParent: false, start: getTextByBlockUid(targetUid).length, }, mutableCursor: true, }); } else { window.roamAlphaAPI.ui.mainWindow .getOpenPageOrBlockUid() .then((uid) => sbBomb({ srcUid: wf.uid, target: { uid: uid || window.roamAlphaAPI.util.dateToPageUid(new Date()), isParent: true, }, mutableCursor: true, }) ); } }, 500); }, }); + labelsThisSync.add(label); + registeredCommandLabels.add(label); }); };Optional: consider extracting the 500 ms delay into a named constant for readability.
555-569: Refresh command is useful; suggest guarding for API availability.Minor: add a defensive check so older Roam builds or test environments without
ui.commandPalettedon’t throw.Example:
- window.roamAlphaAPI.ui.commandPalette.addCommand({ + if (window.roamAlphaAPI.ui?.commandPalette) { + window.roamAlphaAPI.ui.commandPalette.addCommand({ label: REFRESH_SMARTBLOCKS_COMMAND_LABEL, callback: () => { syncCommandPaletteCommands(); renderToast({ id: "smartblocks-command-palette-refresh", intent: Intent.SUCCESS, content: "Command palette workflows refreshed", timeout: 2000, }); }, - }); + }); + }
947-950: Also remove dynamic palette commands on unload.Static commands are removed via the
commandsarray, but dynamic ones won’t be unless you call your remover inunload.Apply:
unload: () => { + removeCommandPaletteCommands(); unloads.forEach((u) => u()); window.clearTimeout(getDailyConfig()["next-run-timeout"]); saveDailyConfig({ "next-run-timeout": 0 }); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.json(1 hunks)src/index.ts(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🧰 Additional context used
🧬 Code graph analysis (1)
src/index.ts (1)
src/utils/core.ts (3)
getCleanCustomWorkflows(356-360)getVisibleCustomWorkflows(341-354)sbBomb(2694-2867)
🔇 Additional comments (5)
src/index.ts (5)
176-181: Sync helper reads clearly and is side‑effect free. LGTM.
183-187: Toggle wiring looks good.
217-229: Opt‑in setting UI/handler looks correct.Switch updates state and triggers a sync as expected.
Please confirm the settings panel persists the “command-palette-opt-in” value automatically (consistent with other switches in this file).
293-295: Good: initialize from saved setting and immediately sync.
433-437: Nice addition: CMD workflow modifier surfaced in cursor menu help.Consistent with the opt‑in behavior.
Summary by CodeRabbit
New Features
Documentation
Chore