Skip to content

feat(nav): link the docs and blog from the app, and the app from the header - #392

Merged
jfrench9 merged 2 commits into
mainfrom
feature/app-docs-blog-links
Sep 17, 2026
Merged

jfrench9 merged 2 commits into
mainfrom
feature/app-docs-blog-links

Conversation

@jfrench9

@jfrench9 jfrench9 commented Sep 17, 2026

Copy link
Copy Markdown
Member

Summary

A signed-in visitor to / is redirected to /home by LandingGate, so they never see the public header's Docs and Blog links, and nothing in the app shell linked to them. A signed-in reader who did reach a docs or blog page saw Login and Register with no way back into the app. This adds Docs and Blog to the end of the sidebar and swaps Login/Register for a single Open app link in the public header when the visitor is signed in.

Changes

  • Sidebar (src/app/(app)/sidebar-config.tsx): two always-visible items after MCP / Repositories: Docs/docs (HiBookOpen) and Blog/blog (HiNewspaper), both with target: '_blank'.
  • Public header (src/components/landing/Header.tsx, used by the landing, docs and blog layouts): desktop and mobile show Open app/home, styled as the Register button, in place of Login and Register once the page is live and the session says the visitor is signed in.
    • Signed-in state comes from the same AuthProvider context LandingGate reads (useOptionalAuth, the non-throwing twin of useAuth, so the header still renders with no provider around it). AuthProvider at the root already checks the session on mount, so the header adds no network request.
    • A hydration guard (useSyncExternalStore, the pattern LandingGate uses) keeps the server HTML identical to today: Login and Register, whatever the session.
  • Tests: src/app/(app)/__tests__/sidebar-config.test.ts (Docs/Blog last, hrefs, target: '_blank', with no graph, a user graph and a repository); src/components/landing/__tests__/Header.test.tsx (signed-out, signed-in, loading, no provider, and server HTML under a signed-in session).

Deploy Notes

None. The branch is on @robosystems/core 0.9.3 (RoboFinSystems/robosystems-core#69), whose CoreSidebar passes target through with rel="noopener noreferrer", so the new links open in a new tab. On 0.9.2 the target was ignored and the link navigated the app's own tab.

Testing

  • env -u NEXT_PUBLIC_* ... NODE_ENV=test npm run test:all: passed (61 test files, 460 tests; format, lint, typecheck, cf-lint clean). The pre-commit hook re-ran format:check, lint, typecheck and tests, also green.
  • npm run build: not run (no routing, layout or config change; Header was already a client component).
  • Not checked in a browser.

🤖 Generated with Claude Code

https://claude.ai/code/session_011KBrhQHzwrB39C4c4Vw9Pq

…header

A signed-in visitor to / is sent to /home, so the public header's Docs and Blog links
were out of reach from inside the app, and a signed-in reader who did land on a docs or
blog page saw Login and Register with no way back. The sidebar now ends with Docs and
Blog, marked to open in a new tab, and the public header swaps Login and Register for a
single Open app link once the page is live and the session says the visitor is signed in.
The server HTML still carries Login and Register, and the session is the one AuthProvider
already checks at the root, so the header adds no request.

Claude-Session: https://claude.ai/code/session_011KBrhQHzwrB39C4c4Vw9Pq
@github-actions github-actions Bot added the change:normal Normal change: automated review documented on the pull request before merge label Sep 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Change classification: normal — platform or application code changed.

A normal change carries a documented automated review on this pull request before merge, in place of a second human approver. The change-classification job requests that review and passes once it has been posted. Recorded by the change-classification job.

@jfrench9

Copy link
Copy Markdown
Member Author

@claude please review this PR

@claude

claude Bot commented Sep 17, 2026

Copy link
Copy Markdown

Claude finished @jfrench9's task in 1m 26s —— View job


Review: feat(nav): link the docs and blog from the app, and the app from the header

  • Gather context (PR body, diff, files)
  • Review src/app/(app)/sidebar-config.tsx
  • Review src/components/landing/Header.tsx
  • Review new tests
  • Post review feedback

Summary: Clean, well-scoped change that closes a real navigation gap (signed-in visitors landing on / never saw Docs/Blog; signed-in readers on docs/blog had no way back). Both files are small, focused diffs with good explanatory comments and solid test coverage. No bugs found.

src/app/(app)/sidebar-config.tsx (+17)

  • Docs/Blog appended after MCP/Repositories, target: '_blank' on both — matches the new sidebar-config.test.ts assertions (ordering, hrefs, icons defined).
  • The PR body is upfront that @robosystems/core 0.9.2's CoreSidebar doesn't forward target to the rendered anchor yet, so these currently open as a full same-tab navigation rather than a new tab. Config is correct for when core catches up — reasonable to land ahead of the core fix rather than block on it.
  • Minor, non-blocking, forward-looking: once core does forward target, consider whether SidebarItemData should also carry rel: 'noopener noreferrer' for target: '_blank' items (reverse-tabnabbing hardening). Not needed today since target isn't rendered yet.

src/components/landing/Header.tsx (+65/-27)

  • useOptionalAuth (non-throwing) + useSyncExternalStore hydration guard mirrors the exact pattern LandingGate.tsx already uses, so server HTML stays deterministic (Login/Register) regardless of session state — verified by the renderToString test.
  • isSignedIn = hydrated && !!auth?.isAuthenticated && !auth.isLoading correctly keeps Login/Register visible while loading and with no provider present (both covered by tests), avoiding a premature "Open app" flash.
  • Desktop and mobile menus both swap consistently; Open app reuses the existing Register button styling as described.
  • No new network request introduced, consistent with the PR's claim (AuthProvider already checks session on mount at the root).

Tests

  • sidebar-config.test.ts: parameterized over no graph / user graph / repository, checks ordering (slice(-4)), hrefs, and target. Good coverage of the graph-dependent branching this touches.
  • Header.test.tsx: signed-out, signed-in, loading, no-provider, and server-HTML-under-signed-in-session cases. This is a thorough set for a hydration-sensitive component — the "no provider" and "still loading" cases are exactly the ones that are easy to regress silently.

Not independently verified: this sandbox has no node_modules installed, so I couldn't run npm run test:all or npm run build myself — relying on the PR's reported results (61 files / 460 tests, format/lint/typecheck/cf-lint clean). Nothing in the diff looks like it would fail typecheck (the target field is already used elsewhere in this same file per the test), but flagging that this review is diff-based, not execution-verified.

No blocking issues. Nice attention to the hydration-mismatch edge case and to matching the existing LandingGate pattern rather than inventing a new one.

@jfrench9
jfrench9 merged commit 12214a0 into main Sep 17, 2026
7 checks passed
@jfrench9
jfrench9 deleted the feature/app-docs-blog-links branch September 17, 2026 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:normal Normal change: automated review documented on the pull request before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant