Add Electron 42 window blur appearance setting#155
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #155 +/- ##
==========================================
+ Coverage 52.56% 53.01% +0.45%
==========================================
Files 177 178 +1
Lines 4429 4493 +64
Branches 920 930 +10
==========================================
+ Hits 2328 2382 +54
- Misses 1903 1911 +8
- Partials 198 200 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Caution Review failedPull request was closed or merged during review WalkthroughこのPRは persisted の windowBackgroundBlurRadius 設定をスキーマ/DEFAULT_SETTINGSで追加し、ブラー用ユーティリティとメインプロセス統合(起動時適用・settings:set ハンドラ)、レンダー側のスライダー UI と App のブラー認識スタイリングを追加します。加えて Markdown Reading Mode の横スクロール防止(CSS/レンダラ)と関連テスト、および少数の依存関係バージョンアップを含みます。 ChangesWindow Background Blur Feature
Markdown Reading Mode Layout Fixes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/services/settings.test.ts (1)
27-34: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
windowBackgroundBlurRadiusの差分比較ケースを明示追加してくださいLine 27 の「primitive差分」テストは
defaultSkillTabのみを変更しており、areSettingsEqualがwindowBackgroundBlurRadiusを比較し忘れた退行を検出できません。blur値だけを変えるケースを1件追加して固定してください。Diff案
+ it('returns false when windowBackgroundBlurRadius differs', () => { + expect( + areSettingsEqual( + { ...baseSettings, windowBackgroundBlurRadius: 0 }, + { ...baseSettings, windowBackgroundBlurRadius: 1 }, + ), + ).toBe(false) + })🤖 Prompt for 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. In `@src/main/services/settings.test.ts` around lines 27 - 34, Add a dedicated test case to verify that areSettingsEqual detects changes to windowBackgroundBlurRadius: create a new it(...) similar to the existing primitive-diff test that calls areSettingsEqual(baseSettings, { ...baseSettings, windowBackgroundBlurRadius: baseSettings.windowBackgroundBlurRadius + 1 }) and assert toBe(false). Reference areSettingsEqual and baseSettings so the test fails if the blur comparison is missing.
🤖 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 `@src/main/ipc/ipc-schemas.test.ts`:
- Around line 130-141: The test for invalid windowBackgroundBlurRadius only
checks a fractional value and an over-max value; add a lower-bound failure case
by asserting schema.safeParse([{ windowBackgroundBlurRadius:
WINDOW_BACKGROUND_BLUR_MIN_RADIUS - 1 }]).success.toBe(false) (use the min
constant paired with WINDOW_BACKGROUND_BLUR_MAX_RADIUS) so the suite covers both
boundary sides; add this expectation alongside the existing safeParse checks in
the same it('rejects an invalid windowBackgroundBlurRadius value', ...) block
referencing schema.safeParse.
In `@src/main/utils/windowBackgroundBlur.test.ts`:
- Around line 12-35: Add unit tests for applyWindowBackgroundBlur that cover
both branches when the BrowserWindow API supports setBackgroundBlur and when it
does not: create minimal mock window objects (one with setBackgroundBlur and one
without) that spy on setBackgroundBlur and setBackgroundColor; call
applyWindowBackgroundBlur with values that exercise clamping (e.g., -12, 12,
WINDOW_BACKGROUND_BLUR_MAX_RADIUS) and with 0 vs non-zero to check background
color selection, then assert that when setBackgroundBlur exists it was called
with the clamped radius and when absent it was not called, and in both cases
assert setBackgroundColor was called with MAIN_WINDOW_OPAQUE_BACKGROUND for
radius 0 and MAIN_WINDOW_BLURRED_BACKGROUND for non-zero.
In `@src/renderer/settings/sections/Appearance.tsx`:
- Around line 52-53: The onChange handler (handleInputChange) currently calls
updateSettings on every slider event causing IPC/save/broadcast storms; change
it so onChange only updates local UI state, and debounce or delay calls to
updateSettings (or call updateSettings only on commit/end interaction) to limit
persistence frequency; implement this by adding a debounced wrapper (e.g.,
useRef + setTimeout or lodash.debounce) around updateSettings or adding an
onChangeCommitted/onMouseUp handler that calls updateSettings once, and ensure
functions named handleInputChange and updateSettings are the only places
modified.
In `@src/renderer/src/components/skills/FileContent.browser.test.tsx`:
- Around line 136-140: The test is scrolling the text node instead of the actual
scroll container; locate the nearest <pre> from the element returned by
screen.getByText(/wide command/) (use .closest('pre')), call scrollTo on that
pre element (not the text node), and update the assertion to verify the pre's
scrollLeft reflects the scroll (e.g., not 0 or equals the expected value) so the
horizontal scroll behavior is correctly tested.
In `@src/renderer/src/styles/globals.css`:
- Around line 131-132: Add a blank line before the "background: transparent;"
declaration to satisfy the stylelint rule declaration-empty-line-before; locate
the block containing "@apply text-foreground font-sans antialiased;" and insert
a single empty line directly above the "background: transparent;" line so the
"background" declaration is separated by one empty line from the previous
declaration.
---
Outside diff comments:
In `@src/main/services/settings.test.ts`:
- Around line 27-34: Add a dedicated test case to verify that areSettingsEqual
detects changes to windowBackgroundBlurRadius: create a new it(...) similar to
the existing primitive-diff test that calls areSettingsEqual(baseSettings, {
...baseSettings, windowBackgroundBlurRadius:
baseSettings.windowBackgroundBlurRadius + 1 }) and assert toBe(false). Reference
areSettingsEqual and baseSettings so the test fails if the blur comparison is
missing.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: d4246f70-4782-4739-8c69-5352cde1b11f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (17)
.storybook/fixtures.tspackage.jsonsrc/main/index.tssrc/main/ipc/ipc-schemas.test.tssrc/main/ipc/ipc-schemas.tssrc/main/ipc/settings.tssrc/main/services/settings.test.tssrc/main/utils/windowBackgroundBlur.test.tssrc/main/utils/windowBackgroundBlur.tssrc/renderer/settings/sections/Appearance.browser.test.tsxsrc/renderer/settings/sections/Appearance.tsxsrc/renderer/src/App.tsxsrc/renderer/src/components/skills/FileContent.browser.test.tsxsrc/renderer/src/components/skills/FileContent.tsxsrc/renderer/src/styles/globals.csssrc/shared/settings.test.tssrc/shared/settings.ts
8e63055 to
0bf7255
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/renderer/settings/sections/Appearance.tsx`:
- Around line 111-113: The effect that syncs blur draft with
windowBackgroundBlurRadius must cancel any pending debounce timer so a stale
pending update doesn't overwrite a broadcasted value; modify the component to
store the pending timer ID (e.g., a ref like blurRadiusTimerRef used by the
slider update logic) and call clearTimeout on that ref inside the
useComponentEffect that runs setBlurRadiusDraft(windowBackgroundBlurRadius), and
also clear/replace the timer whenever the slider starts a new debounce; ensure
the code paths around the slider handler that call updateSettings({
windowBackgroundBlurRadius: ... }) set the ref so it can be cleared by
useComponentEffect to prevent stale updates.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: b5e94bc8-1a47-4b01-974c-a316d478368a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (17)
.storybook/fixtures.tspackage.jsonsrc/main/index.tssrc/main/ipc/ipc-schemas.test.tssrc/main/ipc/ipc-schemas.tssrc/main/ipc/settings.tssrc/main/services/settings.test.tssrc/main/utils/windowBackgroundBlur.test.tssrc/main/utils/windowBackgroundBlur.tssrc/renderer/settings/sections/Appearance.browser.test.tsxsrc/renderer/settings/sections/Appearance.tsxsrc/renderer/src/App.tsxsrc/renderer/src/components/skills/FileContent.browser.test.tsxsrc/renderer/src/components/skills/FileContent.tsxsrc/renderer/src/styles/globals.csssrc/shared/settings.test.tssrc/shared/settings.ts
0bf7255 to
b872bc8
Compare
Summary
Verification
Summary by CodeRabbit