feat(app): collapse notification settings to single tri-state control (phase 2 of #923) by Astro-Han · Pull Request #938 · Astro-Han/pawwork

Astro-Han · 2026-05-27T01:34:27Z

Summary

Replace six sound toggles and three notification switches with a single tri-state Sounds & notifications selector: Never / When not looking / Always. Silence OS notification sound on all Electron/web paths so only the app's own sounds play. Unify settings Select props and fix hover visibility.

Why

Phase 1 (#924) shipped two warm sounds but kept the old six-toggle + three-switch UI from upstream OpenCode. Users had to configure 9 controls for what is really one question: "when do you want to be alerted?" Phase 2 collapses this into a single tri-state that also gates OS notifications, matching the product direction outlined in #923.

Related Issue

Closes #923

Human Review Status

Pending

Review Focus

Migration effect in settings.tsx: old persisted sounds + notifications fields map to the new notify tri-state on first load, then get cleaned up. Verify the mapping covers edge cases (all disabled → "never", any enabled → "unfocused").
Visibility gate: document.hasFocus() is now part of "is the user looking at this session." Confirm this doesn't fire false negatives on macOS when the app window is in a different desktop/space.
Electron silent: true on all three Notification paths (renderer Web API, main process IPC fallback, web entry).

Risk Notes

Migration: in-place on settings.v3 storage key. Users who downgrade to a pre-phase-2 build will lose their notify value and get defaults. Low risk since the old defaults (agent sounds on, error sounds on, notifications on) are more permissive than "unfocused."
Platform: Notification({ silent: true }) is supported on macOS and Windows. Linux behavior depends on the notification daemon; some may ignore the flag. No regression since Linux previously had no sound gating either.

How To Verify

typecheck: 8/8 packages pass (bun run typecheck)
unit (effects): 12 pass, 0 fail (layout-sdk-event-effects.test.ts)
unit (fonts): 2 pass, 0 fail (settings-fonts.test.ts)
manual: settings page shows single tri-state dropdown with working hover
manual: "When not looking" mode — sound fires when window loses focus, silent when focused on the completing session
manual: "Always" mode — sound fires regardless of focus
manual: "Never" mode — no sound, no OS notification
manual: no double system sound on macOS notification

Screenshots or Recordings

Settings page before (6 dropdowns + 3 switches across two sections):

Not captured — the old UI was removed in this PR. See phase 1 PR #924 for the prior state.

Settings page after (single tri-state dropdown):

Verified in bun run dev:desktop — one "Sounds & notifications" row with a "Never / When not looking / Always" dropdown under a "Notifications" section header.

Checklist

How to use this checklist:

Tick a box by replacing [ ] with [x]. Do not edit, add, or remove items.

The bot-applied label items can only be honestly ticked AFTER the PR is opened and the labeler / priority-triage bots have run — return to the PR description and tick them then.

Most items are required. The few that are conditional are explicitly marked (conditional); for those, leave unticked if they truly do not apply and explain why in Risk Notes. All other items must be ticked before requesting human review.

… (phase 2 of #923) Replace six sound toggles and three notification switches with one "Sounds & notifications" selector: Never / When not looking / Always. - settings.tsx: new NotifyLevel type, migration from old persisted shape - notification.tsx: gate idle/error alerts by tri-state + route visibility - layout-sdk-event-effects.ts: unify permission/question notification logic - settings UI: single Select replaces two multi-row sections (-233 LOC) - Electron: silent: true on OS Notification (app owns the sound) - sound.ts: remove unused SOUND_OPTIONS/SoundOption exports - i18n: en + zh updated, old keys removed

- Extract settingsSelectDefaults constant to deduplicate variant/size/ triggerVariant across all four settings dropdowns - Inline notify section into settings-general.tsx, delete standalone settings-sounds-section.tsx - Fix settings Select hover: scope transparent background to non-hover non-expanded state so picker.css hover overlay shows through

The renderer Notification (Web API) and the web entry Notification both lacked silent: true, so the OS played its default alert sound on top of the app's own notify/error sound.

github-actions

Suggested priority: P2 (includes user-path files (packages/app/src/components/settings-general.tsx, packages/app/src/components/settings-notifications-section.tsx, packages/app/src/components/settings-sounds-section.tsx, packages/app/src/context/notification.tsx, packages/app/src/context/settings.tsx, packages/app/src/entry.tsx, packages/app/src/i18n/en.ts, packages/app/src/i18n/zh.ts, packages/app/src/pages/layout.tsx, packages/app/src/pages/layout/layout-sdk-event-effects.test.ts, packages/app/src/pages/layout/layout-sdk-event-effects.ts, packages/app/src/utils/sound.ts, packages/desktop-electron/src/main/ipc.ts, packages/desktop-electron/src/renderer/index.tsx)).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

coderabbitai · 2026-05-27T01:34:44Z

Warning

Review limit reached

@Astro-Han, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 35 minutes and 27 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c14ec413-cb33-4050-a34d-196f1190d6f8

📥 Commits

Reviewing files that changed from the base of the PR and between e936269 and 1b3487e.

📒 Files selected for processing (6)

packages/app/src/components/settings-general.tsx
packages/app/src/context/notification.tsx
packages/app/src/context/settings.tsx
packages/app/src/i18n/zh.ts
packages/app/src/pages/layout/layout-sdk-event-effects.test.ts
packages/app/src/pages/layout/layout-sdk-event-effects.ts

📝 Walkthrough

Walkthrough

Settings are refactored from separate notification and sound toggles to a single tri-state notification-level control ("never" | "unfocused" | "always"). The schema, context API, notification dispatch logic, settings UI, and OS notification handling are updated to use the unified approach, with backward-compatible persistence migration and localized UI strings.

Changes

Notification/Sound Settings Consolidation

Layer / File(s)	Summary
Settings schema and context API `packages/app/src/context/settings.tsx`	Introduces `NotifyLevel` type (`"never" \| "unfocused" \| "always"`), replaces `notifications` and `sounds` sub-objects with single `notify` field in the `Settings` interface, initializes `notify` to `"unfocused"` in defaults, adds a persistence migration effect to convert legacy notification/sound flags to the new level, and exposes `notify.level()` and `notify.setLevel(value)` via the context API.
Notification dispatch with layout integration `packages/app/src/context/notification.tsx`, `packages/app/src/pages/layout/layout-sdk-event-effects.ts`, `packages/app/src/pages/layout.tsx`, `packages/app/src/pages/layout/layout-sdk-event-effects.test.ts`, `packages/app/src/utils/sound.ts`	Updates `handleSessionIdle` and `handleSessionError` to gate alerts by `settings.notify.level()`, consolidates question and permission notification eligibility checks to use the new level with focus-aware suppression (`"unfocused"` suppresses when route is visible), simplifies `SoundID` to an explicit union (`"notify" \| "error"`), updates the layout SDK event effects contract to use `settings.notify.level()` and `effects.playSound(soundID)`, and adjusts test harness inputs and assertions to use the unified notification level.
Settings UI and localization `packages/app/src/components/settings-general.tsx`, `packages/app/src/i18n/en.ts`, `packages/app/src/i18n/zh.ts`	Inlines the notify-level `Select` into `SettingsGeneral`, removes previous `SettingsNotificationsSection` and `SettingsSoundsSection` usage, introduces `settingsSelectDefaults` to standardize `Select` props (variant, size, triggerVariant) across language, color scheme, and theme selections, and updates English and Chinese dictionaries with the new `settings.general.section.notify` section label, title, description, and tri-state option translations.
OS notification silencing and Select styling `packages/app/src/entry.tsx`, `packages/desktop-electron/src/main/ipc.ts`, `packages/desktop-electron/src/renderer/index.tsx`, `packages/ui/src/components/select.css`	Adds `silent: true` to native `Notification` constructors in web and desktop notification handlers to prevent double-sound output, and refines the `Select` settings trigger CSS to scope `background-color: transparent` and `background-image: none` to the `:not(:hover):not([data-expanded])` state for improved visual feedback.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Astro-Han/pawwork#334: Both PRs modify notification dispatch and OS notification behavior in packages/app/src/entry.tsx at the same code level.
Astro-Han/pawwork#461: Both PRs modify Select component settings-trigger CSS styling in packages/ui/src/components/select.css.

Suggested labels

P2, desktop

Poem

🔔 The rabbit heard three different chimes,
And thought, "This cacophony? Not wise!"
One warm bell now at just the right times,
"Never, unfocused, always"—oh my!
The OS stays silent, no double surprise. 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: collapsing notification settings to a single tri-state control as phase 2 of the linked issue `#923`.
Linked Issues check	✅ Passed	All Phase 2 objectives from issue `#923` are addressed: tri-state setting implemented, OS notifications silenced, settings UI refactored, migration logic added, and i18n updated.
Out of Scope Changes check	✅ Passed	All changes are directly aligned with Phase 2 objectives: notification gating, settings consolidation, file deletion, i18n updates, and CSS fixes for the Select component styling.
Description check	✅ Passed	The PR description comprehensively covers all required template sections: Summary, Why, Related Issue, Human Review Status, Review Focus, Risk Notes, How To Verify, Screenshots, and all checklist items are properly addressed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch claude/923-phase2-notify-tristate

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.}

gemini-code-assist

Code Review

This pull request simplifies the notification and sound settings by consolidating them into a single tri-state notify level (never, unfocused, or always). It removes the separate notification and sound settings sections, updates the general settings UI, and introduces a migration effect to map existing user configurations to the new setting. Feedback on these changes highlights reactivity issues in SolidJS: notifyOptions should be wrapped in createMemo to ensure translations update dynamically when the language changes, and the settings migration createEffect should use untrack to prevent unnecessary re-runs when store properties are modified.

"When not looking" should mean the window is backgrounded too, not just a different session route. Add document.hasFocus() guard so switching to another app triggers sounds and notifications.

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/app/src/components/settings-general.tsx`:
- Around line 94-98: notifyOptions is built once so its labels won't update when
locale changes; wrap its construction in a reactive memo (e.g. const
notifyOptions = createMemo(() => [...])) and use notifyOptions() wherever the
Select reads the options (and similarly for the other options at the other
locations). Specifically, replace the static array of { id: NotifyLevel; label:
string } with a createMemo that calls language.t for each label, and update the
Select usage to read notifyOptions() so labels update when language changes.

In `@packages/app/src/i18n/zh.ts`:
- Line 758: The zh locale uses "PawWork" in the translation for the key
"settings.notify.description" which is inconsistent with the rest of the file
that uses "爪印"; update the value for "settings.notify.description" to replace
"PawWork" with "爪印" so the product name is consistently translated across the
locale.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8099e19a-ffcf-42a1-a11b-36971ddb754f

📥 Commits

Reviewing files that changed from the base of the PR and between a689b06 and e936269.

📒 Files selected for processing (15)

packages/app/src/components/settings-general.tsx
packages/app/src/components/settings-notifications-section.tsx
packages/app/src/components/settings-sounds-section.tsx
packages/app/src/context/notification.tsx
packages/app/src/context/settings.tsx
packages/app/src/entry.tsx
packages/app/src/i18n/en.ts
packages/app/src/i18n/zh.ts
packages/app/src/pages/layout.tsx
packages/app/src/pages/layout/layout-sdk-event-effects.test.ts
packages/app/src/pages/layout/layout-sdk-event-effects.ts
packages/app/src/utils/sound.ts
packages/desktop-electron/src/main/ipc.ts
packages/desktop-electron/src/renderer/index.tsx
packages/ui/src/components/select.css

💤 Files with no reviewable changes (2)

packages/app/src/components/settings-notifications-section.tsx
packages/app/src/components/settings-sounds-section.tsx

github-actions · 2026-05-27T01:43:39Z

Perf delta summary

Comparator: pass

Profile / Scenario	interaction median	interaction worst	long task max	tbt	frame gap p95	frame gap max	jank count	cls	status
default / homepage-cold	32 -> 32 (0)	40 -> 40 (0)	74 -> 75 (+1)	24 -> 25 (+1)	33.3 -> 16.8 (-16.5)	183.4 -> 183.3 (-0.1)	4 -> 3 (-1)	0 -> 0 (0)	pass
default / long-session-input-lag	48 -> 48 (0)	48 -> 48 (0)	0 -> 0 (0)	0 -> 0 (0)	16.7 -> 16.7 (0)	16.8 -> 16.8 (0)	0 -> 0 (0)	0 -> 0 (0)	pass
default / session-streaming-long	48 -> 48 (0)	64 -> 64 (0)	0 -> 0 (0)	0 -> 0 (0)	16.8 -> 16.8 (0)	16.8 -> 33.3 (+16.5)	0 -> 0 (0)	0 -> 0 (0)	pass
default / tool-call-expand	40 -> 40 (0)	40 -> 40 (0)	0 -> 0 (0)	0 -> 0 (0)	16.8 -> 16.7 (-0.1)	16.8 -> 16.7 (-0.1)	0 -> 0 (0)	0 -> 0 (0)	pass
default / tool-default-open-heavy-bash	16 -> 16 (0)	16 -> 16 (0)	74 -> 71 (-3)	43 -> 37 (-6)	66.6 -> 50 (-16.6)	133.4 -> 133.3 (-0.1)	4 -> 4 (0)	0.004 -> 0.004 (0)	pass
default / terminal-side-panel-open	40 -> 48 (+8)	48 -> 48 (0)	0 -> 0 (0)	0 -> 0 (0)	33.3 -> 16.8 (-16.5)	33.3 -> 33.3 (0)	0 -> 0 (0)	0 -> 0 (0)	pass
default / session-scroll-reading	16 -> 16 (0)	16 -> 16 (0)	0 -> 0 (0)	0 -> 0 (0)	33.3 -> 16.7 (-16.6)	33.3 -> 16.7 (-16.6)	0 -> 0 (0)	0 -> 0 (0)	pass

…, zh brand name - Wrap notifyOptions in createMemo so labels update on locale change - Wrap migration effect body in untrack to avoid re-triggering on the store writes it performs - Use 爪印 instead of PawWork in zh.ts description (matches the rest of the zh dictionary)

The question notification path only called effects.notify() (now a silent OS notification) without playing the app sound. Add effects.playSound("notify") to match the permission path. Add four tests asserting sound playback for question and permission alerts, and silence for "never" and visible-session cases.

Bump PawWork desktop release version to 2026.5.28. Changes since v2026.5.27: - feat(ui): fold reasoning into trow block (#948) - feat: align outbound HTTP headers with canonical OpenCode desktop (#940) - feat(app): collapse notification settings to single tri-state control (#938) - fix(ui): track List header surface via --list-surface (#954) - fix(ui): render tooltip shortcut hints as plain sans glyphs (#955) - fix(watcher): isolate macOS workspace roots - fix(session): terminalize stale question blockers - fix(session): unify transport error classification for stream disconnect recovery (#941) - test: add route inventory guardrails - ci: repair electron desktop build + install

Astro-Han added 3 commits May 27, 2026 01:44

fix(app): silence system notification sound on all paths

e936269

The renderer Notification (Web API) and the web entry Notification both lacked silent: true, so the OS played its default alert sound on top of the app's own notify/error sound.

Astro-Han added enhancement New feature or request app Application behavior and product flows ui Design system and user interface platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions labels May 27, 2026

github-actions Bot added the P2 Medium priority label May 27, 2026

github-actions Bot reviewed May 27, 2026

View reviewed changes

gemini-code-assist Bot reviewed May 27, 2026

View reviewed changes

Comment thread packages/app/src/components/settings-general.tsx Outdated

Comment thread packages/app/src/components/settings-general.tsx Outdated

Comment thread packages/app/src/context/settings.tsx

fix(app): include window focus in notification visibility check

3e88af9

"When not looking" should mean the window is backgrounded too, not just a different session route. Add document.hasFocus() guard so switching to another app triggers sounds and notifications.

coderabbitai Bot reviewed May 27, 2026

View reviewed changes

Comment thread packages/app/src/components/settings-general.tsx Outdated

Comment thread packages/app/src/i18n/zh.ts Outdated

Astro-Han added 2 commits May 27, 2026 09:54

Astro-Han merged commit 3bf9407 into dev May 27, 2026
27 checks passed

coderabbitai Bot mentioned this pull request May 27, 2026

feat(settings): two-layer takeover shell, migrate pages (PR1 of #604) #951

Merged

Astro-Han mentioned this pull request May 27, 2026

chore(release): bump desktop version to v2026.5.28 #958

Merged

Astro-Han deleted the claude/923-phase2-notify-tristate branch June 2, 2026 08:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(app): collapse notification settings to single tri-state control (phase 2 of #923)#938

feat(app): collapse notification settings to single tri-state control (phase 2 of #923)#938
Astro-Han merged 6 commits into
devfrom
claude/923-phase2-notify-tristate

Astro-Han commented May 27, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

coderabbitai Bot commented May 27, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

❌ Failed checks (1 warning)

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 27, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Astro-Han commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Related Issue

Human Review Status

Review Focus

Risk Notes

How To Verify

Screenshots or Recordings

Checklist

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

❌ Failed checks (1 warning)

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf delta summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Astro-Han commented May 27, 2026 •

edited

Loading

coderabbitai Bot commented May 27, 2026 •

edited

Loading

github-actions Bot commented May 27, 2026 •

edited

Loading