Skip to content

Make Archive TODO hotkey configurable via Roam Hotkeys#11

Closed
salmonumbrella wants to merge 1 commit intoRoamJS:mainfrom
salmonumbrella:feat/configurable-archive-hotkey
Closed

Make Archive TODO hotkey configurable via Roam Hotkeys#11
salmonumbrella wants to merge 1 commit intoRoamJS:mainfrom
salmonumbrella:feat/configurable-archive-hotkey

Conversation

@salmonumbrella
Copy link
Copy Markdown

@salmonumbrella salmonumbrella commented Dec 31, 2025

Summary

  • Replace hardcoded Cmd/Ctrl+Shift+Enter shortcut with Roam's native command palette registration
  • Add "Archive TODO" command to command palette for discoverability
  • Allow users to customize the hotkey via Settings → Hotkeys
  • Set default hotkey to ctrl-shift-enter for backwards compatibility
  • Add discoverability note in TODONT Mode settings description

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Dec 31, 2025

Walkthrough

The PR changes TODONT initialization to accept extensionAPI and uses it to register an "Archive TODO" command via extensionAPI.ui.commandPalette (constant ARCHIVE_COMMAND_LABEL). The previous global keydown listener is removed and replaced by the command palette command; unload now unregisters the command by label. Mode switching now clears previous unloads before setting new ones. The isControl import and an obsolete else block were removed. The UI description string was expanded to mention the "Archive TODO" command and hotkey customization under Settings → Hotkeys.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: making the Archive TODO hotkey configurable through Roam's native hotkeys system instead of hardcoded.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/utils/todont.ts (1)

88-211: Critical: Switching between non-off modes causes duplicate registrations.

When a user switches from one active TODONT mode to another (e.g., from "icon" to "strikethrough"), the existing command registration and observers are not cleaned up before the new ones are created. This leads to:

  • Duplicate command palette entries
  • Multiple observer instances running simultaneously
  • Memory leaks as the unloads Set accumulates cleanup handlers
🔎 Proposed fix

Add cleanup before setting up new registrations when switching modes:

 const initializeTodont = (extensionAPI: OnloadArgs["extensionAPI"]) => {
   const unloads = new Set<() => void>();
   return async (todontMode: typeof TODONT_MODES[number]) => {
     if (todontMode !== "off") {
+      // Clean up previous mode if exists
+      unloads.forEach((u) => u());
+      unloads.clear();
+
       const TODONT_CLASSNAME = "roamjs-todont";
       const css = document.createElement("style");
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8a53f1c and d210e47.

📒 Files selected for processing (2)
  • src/index.ts
  • src/utils/todont.ts
🔇 Additional comments (4)
src/index.ts (2)

21-21: LGTM!

The updated call correctly passes extensionAPI to initializeTodont, enabling the command palette integration.


86-86: Good discoverability improvement!

The updated description clearly informs users about the Archive TODO command and hotkey customization options, improving the feature's discoverability.

src/utils/todont.ts (2)

84-86: LGTM!

The addition of the ARCHIVE_COMMAND_LABEL constant and the updated function signature to accept extensionAPI are well-structured changes that enable the command palette integration.


207-211: Cleanup implementation is sound.

The command removal cleanup is properly structured and will work correctly once the duplicate registration issue is resolved.

Comment on lines +201 to +206
extensionAPI.ui.commandPalette.addCommand({
label: ARCHIVE_COMMAND_LABEL,
callback: todontCallback,
"default-hotkey": "ctrl-shift-enter",
"disable-hotkey": false,
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -n "commandPalette\.addCommand" -A 5 -B 2

Repository: RoamJS/todo-trigger

Length of output: 961


🏁 Script executed:

rg -n "default-hotkey|disable-hotkey" -B 3 -A 3

Repository: RoamJS/todo-trigger

Length of output: 546


🏁 Script executed:

rg -n "ctrl-|shift-|meta-" -i --type ts --type js | head -20

Repository: RoamJS/todo-trigger

Length of output: 132


🏁 Script executed:

find . -name "*.d.ts" -o -name "README*" -o -name "*roam*" -type f | grep -v node_modules | head -20

Repository: RoamJS/todo-trigger

Length of output: 74


🏁 Script executed:

cat package.json | grep -A 5 -B 5 roam

Repository: RoamJS/todo-trigger

Length of output: 549


🏁 Script executed:

rg -n "roamAlphaAPI|roam.*API" -i | head -30

Repository: RoamJS/todo-trigger

Length of output: 1110


🏁 Script executed:

find . -path "*/roamjs-components/*" -name "*.d.ts" | head -20

Repository: RoamJS/todo-trigger

Length of output: 45


🏁 Script executed:

grep -r "commandPalette" . --include="*.d.ts" 2>/dev/null | head -20

Repository: RoamJS/todo-trigger

Length of output: 45


🏁 Script executed:

head -100 README.md

Repository: RoamJS/todo-trigger

Length of output: 3738


🌐 Web query:

Roam Research command palette hotkey format ctrl shift enter API documentation

💡 Result:

Use a plus-separated, lowercase modifier+key string (e.g. "ctrl+shift+enter" or "command+shift+p"). Libraries and apps that Roam integrations use follow this format (modifiers first, joined with "+", then the key). Examples: "ctrl+shift+enter", "ctrl+shift+p". [1][2]

Sources:

  • Roam shortcut listings showing modifier+key notation. [1]
  • npm command-palette component (hotKeys default format "command+shift+p"). [2]
  • TestCafe / common tooling examples using "ctrl+shift+enter" style. [3]

References:
[1] Roam Research keyboard shortcuts (cheatsheet).
[2] @folk-org/react-command-palette (npm) — hotKeys format example.
[3] TestCafe / Stack Overflow example using 'ctrl+shift+enter'.


Fix the hotkey format: use plus signs instead of hyphens.

The hotkey should be "ctrl+shift+enter" not "ctrl-shift-enter". The README documents this for users as "CMD+SHIFT+ENTER (CTRL in windows)", and standard keyboard shortcut notation uses plus signs (+) to separate modifiers and keys. The current hyphen-separated format will likely fail to register the hotkey properly.

🤖 Prompt for AI Agents
In src/utils/todont.ts around lines 201 to 206 the command palette hotkey is
using hyphens ("ctrl-shift-enter") which is incorrect; update the
"default-hotkey" value to use plus signs ("ctrl+shift+enter") so the
modifier/key format matches the README and will register correctly in the UI;
keep the rest of the command object unchanged.

@salmonumbrella
Copy link
Copy Markdown
Author

salmonumbrella commented Feb 14, 2026

@mdroidian Just checking in on this PR. It's been open for a while and looks ready to merge.

I've also just opened #13 which fixes #4, #8, and #10 (TODO transition handling bugs). Would be great if you could review and merge both when you get a chance. Thanks!

@mdroidian
Copy link
Copy Markdown
Contributor

Hi — thank you for putting this together. I appreciate the effort here.

We just added a formal contributing guide, so this likely wasn’t visible when you started the PR. Going forward, we’re trying to standardize reviews around it to keep things manageable:

https://github.com/RoamJS/contributing/blob/main/contributing.md

A couple adjustments are needed before this can move forward:

  • Include a short (2–3 min) Loom (or other) walkthrough showing what changed and how you verified it.
  • via the loom video, verifying each point in the proposed "Test Plan"

Once those are complete, please tag me in a comment or as a reviewer and I’ll be happy to take a look.

@salmonumbrella salmonumbrella force-pushed the feat/configurable-archive-hotkey branch from 88ca790 to cc5c50f Compare February 22, 2026 07:44
@salmonumbrella
Copy link
Copy Markdown
Author

Consolidated into the integration/selective-todont branch — configurable hotkey is included in the new combined PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants