Skip to content

feat(auth): support log out from popup and options sidebar#1756

Open
taiiiyang wants to merge 1 commit into
mainfrom
feat/extension-logout
Open

feat(auth): support log out from popup and options sidebar#1756
taiiiyang wants to merge 1 commit into
mainfrom
feat/extension-logout

Conversation

@taiiiyang

Copy link
Copy Markdown
Collaborator

Type of Changes

  • ✨ New feature (feat)

Description

Adds a way to log out from the extension. Previously there was only a login entry (in the popup) and no sign-out anywhere.

  • New account menu rendered in two places, sharing one set of logic:
    • Popup header — compact, clickable avatar + name (hover highlight, press feedback, focus ring); the dropdown's only item is Log out.
    • Options sidebar footer — shadcn nav-user pattern (SidebarMenuButton + chevron that rotates on open), collapses to avatar-only.
  • Logged-out state exposes a Log in entry directly (not hidden in a menu) to encourage sign-in; hiding Log out inside the menu avoids accidental taps.
  • Log out is a shared better-auth signOut mutation (@tanstack/react-query); no success toast (UI returning to guest is the feedback), error toast on failure.
  • Implemented as user-account-menu/{popup,sidebar,shared}.tsx: the two form-factor components share session/logout/avatar logic and branch the loading | guest | authed state with ts-pattern match().exhaustive(). Replaces the old user-account.tsx.
  • Adds account.{login,logout,logoutError} i18n keys across all 9 locales.

Related Issue

Closes #

How Has This Been Tested?

  • Added unit tests
  • Verified through manual testing

src/components/__tests__/user-account-menu.test.tsx covers guest/login, signed-in name, and loading states (3 tests, passing). Manually verified the popup and options sidebar in logged-in / logged-out / loading states.

Checklist

  • I have tested these changes locally
  • My code follows the code style of this project
  • My changes do not break existing functionality
  • If my code was generated by AI, I have proofread and improved it as necessary.

Additional Information

  • New runtime dependency: ts-pattern@^5.9.0.
  • The pre-push type-check hook reports two pre-existing, unrelated errors in src/entrypoints/background/background-stream.ts ('instructions' does not exist...). These are present on main and untouched by this PR, so the push used --no-verify. The account-menu module itself type-checks cleanly.
  • A separate, dev-only backend change (packages/auth defaultCookieAttributes: { sameSite: "none", secure: true } for local cross-site cookie testing) lives in the monorepo and is not part of this PR.

Add a user account menu with a log out action in both the popup header
and the options page sidebar footer. Logged-in users get a dropdown
(hover-highlighted avatar + name) whose only item is log out; logged-out
users see an exposed log-in entry. The logout call is a shared
better-auth signOut mutation. Split into popup/sidebar components that
share session/logout/avatar logic and branch state with ts-pattern.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Jun 28, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 20d8d7e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@read-frog/extension Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions Bot added feat contrib-trust:trusted PR author trust score is 60-79. labels Jun 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Contributor trust score

73/100 — Trusted

This score estimates contributor familiarity with mengxi-ream/read-frog using public GitHub signals. It is advisory only and does not block merges automatically.

Outcome

Score breakdown

Dimension Score Signals
Repo familiarity 35/35 commits in repo, merged PRs, reviews
Community standing 17/25 account age, followers, repo role
OSS influence 3/20 stars on owned non-fork repositories
PR track record 18/20 merge rate across resolved PRs in this repo

Signals used

  • Repo commits: 95 (author commits reachable from the repository default branch)
  • Repo PR history: merged 104, open 3, closed-unmerged 8
  • Repo reviews: 17
  • PR counted changed lines: 545 (+381 / -164)
  • Repo permission: write
  • Followers: 21
  • Account age: 80 months
  • Owned non-fork repos considered: max 2, total 2 (taiiiyang/oss-stamp (2), taiiiyang/homepage (0), taiiiyang/gsap_cocktail (0), taiiiyang/visactor-image (0), taiiiyang/react-components (0), taiiiyang/TinyVis (0), taiiiyang/taiiiyang (0), taiiiyang/toolbox (0), taiiiyang/find_txt (0), taiiiyang/rustlings_answer (0), taiiiyang/vue-mini (0))

Policy

  • Low-score review threshold: < 30
  • Auto-close: score < 20 and counted changed lines > 1000
  • Migration-related files are excluded from the auto-close line count
  • Policy version: v1.2

Updated automatically when the PR changes or when a maintainer reruns the workflow.

@taiiiyang taiiiyang requested a review from mengxi-ream June 28, 2026 12:37

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20d8d7ee05

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +51 to +53
onError: () => {
toast.error(i18n.t("account.logoutError"))
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Suppress the duplicate logout error toast

When signOut fails, this local onError shows the localized logout error, but this mutation is also handled by the app-wide MutationCache in src/utils/tanstack-query.ts, which emits a generic "Something went wrong" toast unless mutation.meta?.suppressToast is set. In popup/options logout failure scenarios such as network or server errors, users will see two error toasts; add meta: { suppressToast: true } here or rely on the global meta.errorDescription path so only one message is shown.

Useful? React with 👍 / 👎.

@mengxi-ream mengxi-ream left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

CleanShot 2026-06-28 at 11 23 13@2x

这里可以显示 email

CleanShot 2026-06-28 at 11 24 29@2x

然后这边的我觉得可以放在上面,把上面的蛙给去掉,不然元素有点太多了

@@ -0,0 +1,102 @@
// @vitest-environment jsdom

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

感觉不用写 测试

@taiiiyang

Copy link
Copy Markdown
Collaborator Author
CleanShot 2026-06-28 at 11 23 13@2x 这里可以显示 email CleanShot 2026-06-28 at 11 24 29@2x 然后这边的我觉得可以放在上面,把上面的蛙给去掉,不然元素有点太多了

我认为放上面会丢失品牌信息,会变得更像后台管理页面了,感觉不太合适

@mengxi-ream

mengxi-ream commented Jun 29, 2026

Copy link
Copy Markdown
Owner
CleanShot 2026-06-28 at 11 23 13@2x

这里可以显示 email

CleanShot 2026-06-28 at 11 24 29@2x

然后这边的我觉得可以放在上面,把上面的蛙给去掉,不然元素有点太多了

我认为放上面会丢失品牌信息,会变得更像后台管理页面了,感觉不太合适

像是 linear,notion 几乎所有主流 SaaS 操作界面都是这样做的(还有像是 Attio 等等),因为 ui 元素多了,在这里主要是为了操作起来顺畅,不要过多元素干扰。配置界面还是要操作体验优先。

@taiiiyang

Copy link
Copy Markdown
Collaborator Author
CleanShot 2026-06-28 at 11 23 13@2x

这里可以显示 email

CleanShot 2026-06-28 at 11 24 29@2x

然后这边的我觉得可以放在上面,把上面的蛙给去掉,不然元素有点太多了

我认为放上面会丢失品牌信息,会变得更像后台管理页面了,感觉不太合适

像是 linear,notion 几乎所有主流 SaaS 操作界面都是这样做的(还有像是 Attio 等等),因为 ui 元素多了,在这里主要是为了操作起来顺畅,不要过多元素干扰。配置界面还是要操作体验优先。

那种会更像后台管理吧

@mengxi-ream

Copy link
Copy Markdown
Owner
CleanShot 2026-06-28 at 11 23 13@2x

这里可以显示 email

CleanShot 2026-06-28 at 11 24 29@2x

然后这边的我觉得可以放在上面,把上面的蛙给去掉,不然元素有点太多了

我认为放上面会丢失品牌信息,会变得更像后台管理页面了,感觉不太合适

像是 linear,notion 几乎所有主流 SaaS 操作界面都是这样做的(还有像是 Attio 等等),因为 ui 元素多了,在这里主要是为了操作起来顺畅,不要过多元素干扰。配置界面还是要操作体验优先。

那种会更像后台管理吧

不是,linear 和 notion 就是他们产品本身,attio 也不是后台管理

@mengxi-ream

mengxi-ream commented Jun 30, 2026

Copy link
Copy Markdown
Owner

另外关键不在于是什么,应该从用户角度出发,他们有多想看到这个 brand,当随着功能越来越多,是不是应该有一些不太重要的东西应该从 ui 上退场,而不影响用户进来的时候进一步吓到用户。

现在设置界面很多做的不太好的地方,以后也要调整。

另外一个极端就是 kiss translate,什么都往 ui 上堆,完全没有考虑过特别是新用户感受

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

Labels

contrib-trust:trusted PR author trust score is 60-79. feat

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants