Skip to content

fix(nav): add in-app back button and prevent LAST_DOC_ID from drifting on every navigation - #67

Merged
PeterGuy326 merged 3 commits into
bytefolk:mainfrom
sun-970:fix/back-navigation-and-entry-doc-persistence
Sep 18, 2026
Merged

PeterGuy326 merged 3 commits into
bytefolk:mainfrom
sun-970:fix/back-navigation-and-entry-doc-persistence

Conversation

@sun-970

@sun-970 sun-970 commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add in-app back button to the TopBar (ArrowLeft icon next to Logo) that calls window.history.back(). The button only appears after the first in-app navigation to avoid confusion on direct page load.
  • Fix LAST_DOC_ID persistence using sessionStorage to track whether we've recorded the entry doc. This properly scopes the flag to the page session — survives SPA navigation but resets on full page reload.
  • Add i18n keys for the back button label (goBack) in zh-cn.json and en.json.

Test plan

  • npx vitest run — all tests pass
  • npx tsc --noEmit — no TypeScript errors
  • Back button is hidden on direct page load
  • Back button appears after in-app navigation and works correctly
  • LAST_DOC_ID is written only on first doc load per session
  • "Get Started" button returns to the entry doc after in-session navigation

Note

This PR depends on #65 (i18n base). Should be rebased after #65 is merged.

Closes: #66

@PeterGuy326 PeterGuy326 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

REQUEST CHANGES\n\n1. src/app/[locale]/work/[id]/(content)/content-for-my-doc.tsx:22-42 使用模块级可变变量 hasRecordedEntryDoc,但注释称它是 page session。这个 flag 会跟随浏览器 bundle 跨组件挂载、同一 SPA 中离开文档页再进入另一篇文档,以及 HMR 生命周期持续,实际不是一次页面会话;在 SSR/预渲染或模块复用场景也需要确认不会提前消耗。请改为组件实例或明确的页面会话状态,并补充首次进入、离开后再次进入、侧边栏切换的回归测试。\n\n2. src/app/[locale]/work/[id]/top-bar.tsx:42-47 返回按钮无可退历史判断。首次直接打开文档时按钮仍显示,点击会退到站外页面或无效果。请基于现有 pushState/popstate 记录判断是否存在应用内历史,没有时隐藏或 disabled,并补充 direct-load 与 in-app navigation 的手工/自动验证。\n\n验证:PR head 33cffac 已在本地核对,npx vitest run src/tests/lib/dt.test.ts 通过,3 tests;GitHub CI 当前全绿,但 PR 描述中的 4 个导航手工场景仍未完成,本次 diff 也没有 CHANGELOG.md 的 [Unreleased] 条目。\n\n跨 PR:#67 当前包含 #65 的 commit b3f1749,并改动相同的 i18n 文件。建议先合并并稳定 #65,再让 #67 基于 #65 重放/去重后复审,避免把两个 PR 的基础改动和冲突混在一起。

@PeterGuy326 PeterGuy326 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

复核当前 head 34e7867:模块级 flag 已改为 sessionStorage,返回按钮也已加应用内历史判断,现有 CI 全绿;本次先不 APPROVE,剩余门禁仍未闭环:PR 描述还写着 module-level flag,307 测试数和手工验证清单未更新/未勾选,本次 diff 仍没有 CHANGELOG.md 的 [Unreleased] 条目;同时该分支仍包含 #65 的基础提交,建议 #65 稳定后 rebase/去重再 request re-review。

@waterbro-8 waterbro-8 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review

CI: test and docker-build pass.

The back button + LAST_DOC_ID session flag address the chromeless-container stranding in #66. Using existing pushState/popstate is the right mechanism.

Must fix

  1. This PR also contains #65. timeAgo i18n, version createdAt ISO, version-dialog locale formatting, and dt.ts changes duplicate #65. That will conflict on merge and makes review/revert messy. Please drop those files from this branch and leave them on #65.
  2. src/lib/dt.ts imports useTranslations and adds useTimeAgo. If any leftover i18n stays here, a React hook inside a shared date lib is easy to call from a server module. Prefer keeping hooks out of dt.ts.

Gaps vs #66 (non-blocking if you update the issue)

  • No ancestor breadcrumb.
  • No in-app “reset entry point” control (issue AC). sessionStorage flag is undocumented.
  • Back button only appears after an in-app EVENT_KEY_NAV_DOC; a deep link still has no way back.

Verdict: request changes — strip the #65 overlap, then this is mergeable for the back-button slice.

@sun-970

sun-970 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Changes pushed: stripped #65 overlap (rebased onto main), CHANGELOG entry added, CI green. Ready for re-review.

@waterbro-8 waterbro-8 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re-review

Previous must-fixes look done on the current head:

  • #65 overlap is gone (this diff is TopBar + entry-doc flag + i18n goBack + CHANGELOG only)
  • module-level flag replaced with sessionStorage
  • back button is gated on in-app EVENT_KEY_NAV_DOC
  • CHANGELOG [Unreleased] is present
  • CI is green

Residual (non-blocking)

  • hasRecordedEntryDoc is a magic string; match LAST_DOC_ID_KEY and keep it in constants.
  • Still no automated test for direct-load vs in-app navigation. Manual checklist in the PR body is the only coverage.
  • history.back() after EVENT_KEY_NAV_DOC is enough for the chromeless-container slice. Ancestor breadcrumb and an explicit “reset entry” control from #66 remain out of this PR.

Verdict: approve for the back-button / entry-doc slice.

PeterGuy326
PeterGuy326 previously approved these changes Sep 18, 2026

@PeterGuy326 PeterGuy326 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed current head 8654fb2.

  • The previously requested module-level flag change is now sessionStorage-scoped.
  • The back button is gated by the in-app EVENT_KEY_NAV_DOC navigation event, so direct loads do not expose it.
  • The #65 overlap is gone from the current diff; only the back-button/entry-doc slice, locale labels, and CHANGELOG entry remain.
  • Required CI is green: test, docker-build, dependency review, and Scorecard.
  • Existing CODEOWNER approval is present.

Residuals are non-blocking for this slice: the sessionStorage key could be centralized, and navigation behavior is covered by the documented manual checklist rather than an automated regression. Because this PR depends on #65, rebase or otherwise verify the final merge result after #65 lands.

@waterbro-8

Copy link
Copy Markdown

#65 已合入 main3bd2e27)。这个 PR 现在与 main 冲突(多半是 CHANGELOG)。我推不了 sun-970/doc,请 rebase 到当前 main 后再 push。

user added 3 commits September 18, 2026 16:51
…g on every navigation

- Add a back button (ArrowLeft icon) to the TopBar breadcrumbs slot that calls
  window.history.back(), leveraging the existing pushState/popstate infrastructure
  in @directory/util.ts for in-app navigation without browser chrome
- Change LAST_DOC_ID persistence to only write on the first doc load per page
  session, preventing in-session sidebar navigation from silently overwriting the
  entry point that "Get Started" and external hosts rely on
- Add i18n keys for the back button label (goBack) in zh-cn and en

Closes: bytefolk#66
…on direct load

- Replace module-level hasRecordedEntryDoc with sessionStorage to properly
  scope the flag to the page session (survives SPA nav, resets on reload)
- Hide back button until first in-app navigation to avoid confusion on
  direct page load with no history
@sun-970
sun-970 force-pushed the fix/back-navigation-and-entry-doc-persistence branch from 8654fb2 to b68710f Compare September 18, 2026 08:52

@Bindy-lbb Bindy-lbb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. Clean approach: entry doc recorded once per session via sessionStorage gate, back button gated on emitter-tracked in-app navigation so it only appears after a genuine SPA transition. i18n keys present for both locales. No concerns.

@PeterGuy326

Copy link
Copy Markdown
Contributor

@sun-970 #65 已合入 main,现在 #67 与最新 main 冲突,GitHub 自动 rebase 无法自动处理

我这边没有 sun-970/doc 的 push 权限,麻烦你把当前分支 rebase 到 bytefolk/doc/main 后 force-with-lease 推回,CHANGELOG 里的两个 Unreleased 条目都保留

推好后告诉我,我继续合 #67#69
#67

@PeterGuy326
PeterGuy326 merged commit 96dd85e into bytefolk:main Sep 18, 2026
5 checks passed
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.

No in-app back-navigation and LAST_DOC_ID silently overwrites entry point in embedded containers

4 participants