Repository navigation
Fix manul install, make website install sections linkable - #230
Conversation
- Updated README.md, FAQ.md, and docs/index.html - Changed '@wonderwhy-er/desktop-commander' to '@wonderwhy-er/desktop-commander@latest' - Ensures manual installations always use the latest version from npm
- Updated openTab() function to update URL hash when switching tabs - Enhanced handleAnchorNavigation() to handle all installation tab anchors - Now all installation links work: #npx, #bash, #smithery, #manual, #local, #docker-install - URL updates when changing tabs, making links shareable - Maintains special handling for docker-install with highlighting
- Added handling for #installation link that scrolls to section without changing tabs - Maintains default tab (NPX Install) when using general installation link - Existing tab-specific links still work (#npx, #bash, #smithery, etc.) - Navigation menu 'Installation' link now works properly
- Modified smooth scrolling code to update URL hash using history.pushState() - Now clicking 'Installation' in header properly updates URL to #installation - Maintains smooth scrolling behavior while enabling shareable URLs - All navigation links now update the URL hash as expected
WalkthroughDocumentation examples now pin the NPX package to @latest. The documentation site’s tab/hash navigation logic was updated to synchronize URL hashes with tab state and handle installation anchors. Test expectation files were adjusted for REPL outputs and values. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant B as Browser
participant JS as docs/js/main.js
participant DOM as Document
participant H as History API
rect rgb(245,248,255)
note right of U: Page load
U->>B: Open docs page with URL (may include hash)
B->>JS: DOMContentLoaded
JS->>DOM: Read current hash
JS->>JS: Map hash to installation tab (if any)
alt Valid installation anchor
JS->>DOM: Scroll to installation section
JS->>DOM: Programmatically click matching tab
JS->>H: replaceState to reflect tab/hash
else No matching anchor
JS->>DOM: No tab change
end
end
rect rgb(242,255,245)
note right of U: User navigates
U->>DOM: Click tab button
JS->>H: replaceState with tab hash (docker -> #docker-install)
JS->>DOM: Activate tab content
end
rect rgb(255,249,242)
note right of U: In-page anchor click
U->>DOM: Click anchor (#npx/#manual/...)
JS->>DOM: querySelector(anchor)
JS->>DOM: Scroll to target
JS->>H: pushState with new hash
JS->>DOM: If maps to tab, activate tab
end
rect rgb(253,242,255)
note over B,JS: Hash change
B-->>JS: hashchange
JS->>DOM: Apply mapped tab/scroll behavior
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
README.md (1)
146-146: Duplicate heading markers."### ### Option 5" has a doubled "###".
-### ### Option 5: Checkout locally ❌ **Manual Updates** **Requires Node.js** ❌ **Manual Updates** **Requires Node.js** +### Option 5: Checkout locally ❌ **Manual Updates** **Requires Node.js**test/test_output/repl_test_output.txt (1)
6-6: Make expected-output files deterministic.These values come from Math.random() in tests; committing specific numbers will churn. Ensure tests don’t diff against this file verbatim, or strip randomness before persisting.
Would you like a patch that writes stable placeholders (e.g., "REPL_TEST_VALUE: ") and updates assertions accordingly?
Also applies to: 14-14
docs/index.html (2)
1476-1476: Good pin. Clarify Windows command variant.Add a short comment or footnote that Windows users should use "npx.cmd".
"-y", - "@wonderwhy-er/desktop-commander@latest" + "@wonderwhy-er/desktop-commander@latest" + // On Windows, set "command": "npx.cmd"
1341-1406: Deep-Linking & URL Hash Updates Confirmed
The installation tabs correctly activate based on direct hash values and clicking tabs updates the URL as intended.• In docs/js/main.js, the openTab function (lines 69–76) calls history.replaceState(null, null, '#'+tabName) (or '#docker-install' for Docker) to update the URL hash when a tab is clicked.
• The handleAnchorNavigation routine (lines 442–521) runs on DOMContentLoaded and on hashchange, mapping hashes ‘npx’, ‘bash’, ‘smithery’, ‘manual’, ‘local’, ‘docker’, and ‘docker-install’ to their respective tabs and scrolling the installation section into view.Optional consideration: the code does not parse compound hashes like #installation&tab=manual; if that syntax is required, extend handleAnchorNavigation to split the hash at ‘&’ and extract the tab parameter, or update any documentation/examples to link directly via #manual.
test/test_output/node_repl_debug.txt (1)
39-41: Normalize debug logs (strip ANSI and unify prefixes) for stable artifacts.The last three lines dropped “[STDOUT]” while others keep it; mixed styles + ANSI codes make diffs noisy. Strip ANSI and consistently prefix lines when writing the debug file.
Proposed change in test/test-node-repl.js (outside this file):
// at top import stripAnsi from 'strip-ansi'; // or use regex: text.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '') // replace log() to normalize before persisting function log(message) { const clean = stripAnsi(message); console.log(message); debugLog += clean + '\n'; } // when appending raw stream chunks, prefix consistently node.stdout.on('data', (data) => { const text = data.toString().trim(); outputBuffer += text + '\n'; log(`[STDOUT] ${text}`); });docs/js/main.js (3)
461-474: Normalize hash; make matching robust and explicit.Handle mixed-case/encoded hashes and use in-operator for clarity.
- const hash = window.location.hash.substring(1); // Remove the # + const rawHash = window.location.hash.substring(1); // Remove the # + const hash = decodeURIComponent(rawHash).toLowerCase(); @@ - if (validTabs[hash]) { + if (hash in validTabs) {Optionally hoist the installation anchor list into a top-level constant to avoid duplication with the click handler.
479-484: Brittle selector: relying on inline onclick substring.Prefer a stable data attribute, with onclick fallback for backward compat.
- const tabButton = document.querySelector(`[onclick*="${targetTab}"]`); + const tabButton = + document.querySelector(`[data-tab="${targetTab}"]`) || + document.querySelector(`[onclick*="${targetTab}"]`);Remember to add data-tab="" to the tab buttons in HTML.
492-509: Comment/code mismatch: UTM vars read but not used; highlight always fires.Gate the highlight on UTM presence as described.
- if (hash === 'docker-install') { - // Check for UTM parameters to confirm this came from Docker Gateway - const urlParams = new URLSearchParams(window.location.search); - const utmSource = urlParams.get('utm_source'); - const utmMedium = urlParams.get('utm_medium'); - const utmCampaign = urlParams.get('utm_campaign'); - - // Highlight the docker section briefly - const dockerSection = document.getElementById('docker'); - if (dockerSection) { - dockerSection.style.transition = 'background-color 0.5s ease'; - dockerSection.style.backgroundColor = '#e3f2fd'; - setTimeout(() => { - dockerSection.style.backgroundColor = ''; - }, 2000); - } - } + if (hash === 'docker-install') { + const urlParams = new URLSearchParams(window.location.search); + const hasUtm = ['utm_source','utm_medium','utm_campaign'].some(k => urlParams.has(k)); + if (hasUtm) { + const dockerSection = document.getElementById('docker'); + if (dockerSection) { + dockerSection.style.transition = 'background-color 0.5s ease'; + dockerSection.style.backgroundColor = '#e3f2fd'; + setTimeout(() => { + dockerSection.style.backgroundColor = ''; + }, 2000); + } + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
FAQ.md(1 hunks)README.md(1 hunks)docs/index.html(1 hunks)docs/js/main.js(4 hunks)test/test_output/node_repl_debug.txt(1 hunks)test/test_output/repl_test_output.txt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
test/test_output/repl_test_output.txt (1)
test/test-repl-interaction.js (3)
runTests(215-240)testNodeREPL(143-210)testPythonREPL(67-138)
FAQ.md (1)
setup-claude-server.js (1)
setup(614-854)
README.md (1)
setup-claude-server.js (1)
setup(614-854)
test/test_output/node_repl_debug.txt (1)
test/test-node-repl.js (1)
testNodeREPL(36-151)
docs/js/main.js (1)
docs/js/analytics-tracking.js (2)
section(388-396)initializeNavigationTracking(386-398)
🪛 LanguageTool
test/test_output/node_repl_debug.txt
[grammar] ~41-~41: There might be a mistake here.
Context: ..., User 1! Hello, User 2! undefined >�[0m �[32m[STDOUT] >�[0m �[34mFinal output bu...
(QB_NEW_EN)
🔇 Additional comments (2)
docs/js/main.js (2)
69-76: LGTM: hash sync on tab switch (docker → #docker-install).Consistent, shareable anchors without polluting history.
447-460: LGTM: dedicated handling for #installation.Scroll-only behavior matches intent and avoids unintended tab switches.
| anchor.addEventListener('click', function (e) { | ||
| e.preventDefault(); | ||
|
|
||
| const target = document.querySelector(this.getAttribute('href')); | ||
| const href = this.getAttribute('href'); | ||
| const target = document.querySelector(href); | ||
| if (target) { | ||
| // Extra offset for section IDs that are inside the cases section | ||
| const isSubsection = this.getAttribute('href').startsWith('#cases-'); | ||
| const isSubsection = href.startsWith('#cases-'); | ||
| const offset = isSubsection ? 120 : 80; | ||
|
|
||
| // Update URL hash to match the clicked link | ||
| history.pushState(null, null, href); | ||
|
|
There was a problem hiding this comment.
In-page install anchors won’t switch tabs (pushState doesn’t fire hashchange; no target for #docker-install).
Clicking links like #npx or #docker-install won’t run handleAnchorNavigation because:
- You preventDefault and use history.pushState (no hashchange event).
- querySelector(href) returns null for #docker-install, so URL isn’t updated and no fallback runs.
Call handleAnchorNavigation for installation-related anchors and always update the URL, even if no DOM target exists.
Apply:
anchor.addEventListener('click', function (e) {
e.preventDefault();
-
- const href = this.getAttribute('href');
- const target = document.querySelector(href);
- if (target) {
- // Extra offset for section IDs that are inside the cases section
- const isSubsection = href.startsWith('#cases-');
- const offset = isSubsection ? 120 : 80;
-
- // Update URL hash to match the clicked link
- history.pushState(null, null, href);
-
- window.scrollTo({
- top: target.offsetTop - offset,
- behavior: 'smooth'
- });
- }
+ const href = this.getAttribute('href');
+ const hash = href.startsWith('#') ? href.slice(1) : href;
+
+ // Installation anchors: delegate to router so tabs sync correctly
+ if (['installation','npx','bash','smithery','manual','local','docker','docker-install'].includes(hash)) {
+ history.pushState(null, null, href);
+ handleAnchorNavigation();
+ return;
+ }
+
+ // Regular in-page anchors
+ const target = document.querySelector(href);
+ // Always reflect the anchor in the URL
+ history.pushState(null, null, href);
+ if (target) {
+ const isSubsection = href.startsWith('#cases-');
+ const offset = isSubsection ? 120 : 80;
+ window.scrollTo({
+ top: target.offsetTop - offset,
+ behavior: 'smooth'
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| anchor.addEventListener('click', function (e) { | |
| e.preventDefault(); | |
| const target = document.querySelector(this.getAttribute('href')); | |
| const href = this.getAttribute('href'); | |
| const target = document.querySelector(href); | |
| if (target) { | |
| // Extra offset for section IDs that are inside the cases section | |
| const isSubsection = this.getAttribute('href').startsWith('#cases-'); | |
| const isSubsection = href.startsWith('#cases-'); | |
| const offset = isSubsection ? 120 : 80; | |
| // Update URL hash to match the clicked link | |
| history.pushState(null, null, href); | |
| anchor.addEventListener('click', function (e) { | |
| e.preventDefault(); | |
| const href = this.getAttribute('href'); | |
| const hash = href.startsWith('#') ? href.slice(1) : href; | |
| // Installation anchors: delegate to router so tabs sync correctly | |
| if (['installation', 'npx', 'bash', 'smithery', 'manual', 'local', 'docker', 'docker-install'].includes(hash)) { | |
| history.pushState(null, null, href); | |
| handleAnchorNavigation(); | |
| return; | |
| } | |
| // Regular in-page anchors | |
| const target = document.querySelector(href); | |
| // Always reflect the anchor in the URL | |
| history.pushState(null, null, href); | |
| if (target) { | |
| const isSubsection = href.startsWith('#cases-'); | |
| const offset = isSubsection ? 120 : 80; | |
| window.scrollTo({ | |
| top: target.offsetTop - offset, | |
| behavior: 'smooth' | |
| }); | |
| } | |
| }); |
| "@wonderwhy-er/desktop-commander@latest" | ||
| ] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Good: pin to @latest. Add Windows note for npx.
Mirror the README guidance: on Windows, "command" should be "npx.cmd" to avoid spawn issues.
"command": "npx",
+ // On Windows use "npx.cmd"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In FAQ.md around lines 144-145, the example shows installing
"@wonderwhy-er/desktop-commander@latest" but lacks a Windows-specific note;
update the FAQ to mirror the README by adding a short Windows instruction:
mention that when running via npx on Windows the command should be invoked as
"npx.cmd" (e.g., use npx.cmd <package> ...) to avoid spawn issues, and include
this note adjacent to or in parentheses after the existing example so Windows
users know to use npx.cmd.
| "args": [ | ||
| "-y", | ||
| "@wonderwhy-er/desktop-commander" | ||
| "@wonderwhy-er/desktop-commander@latest" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pinning to @latest via npx is good. Also document Windows-specific command.
Consider adding a Windows note to use "npx.cmd" in Claude config to match your setup script behavior; otherwise Windows users may fail to launch.
"command": "npx",
+ // On Windows use "npx.cmd"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@wonderwhy-er/desktop-commander@latest" | |
| "command": "npx", | |
| // On Windows use "npx.cmd" |
🤖 Prompt for AI Agents
In README.md around line 134, add a short Windows-specific note explaining that
when invoking the package via npx in Windows environments the command should use
"npx.cmd" (e.g., in Claude config or any Windows scripts) to match the setup
script behavior; update the example/usage section near the existing
"@wonderwhy-er/desktop-commander@latest" line to include the Windows variant and
a one-line explanation so Windows users know to replace "npx" with "npx.cmd" to
avoid launch failures.
wonderwhy-er
changed the title
1 participant
Summary by CodeRabbit
New Features
Documentation
Tests