Skip to content

fix: preserve remaining duration when paused repeatedly - #417

Merged
zombieJ merged 1 commit into
masterfrom
codex/notification-repeated-pause
Sep 30, 2026
Merged

zombieJ merged 1 commit into
masterfrom
codex/notification-repeated-pause

Conversation

@zombieJ

@zombieJ zombieJ commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Summary

Moving between stacked notices can pause an already paused timer again. This incorrectly counts time spent hovering while the progress bar remains frozen, causing notices to close immediately when the pointer leaves the list.

Clear the timer timestamp after pausing so repeated pauses preserve the remaining duration. Add a regression test covering repeated pauses, frozen progress, and closing only after the remaining duration elapses.

Validation

  • Regression test fails before the fix and passes afterward.
  • All 56 tests pass.
  • TypeScript and Prettier checks pass.
  • ESLint passes with existing warnings.

Summary by CodeRabbit

  • Bug Fixes

    • 修复通知多次暂停后恢复时的计时问题,确保剩余时长计算准确,并在到期时正常关闭通知。
  • Tests

    • 增加通知多次暂停、恢复及到期行为的测试覆盖。

@vercel

vercel Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
notification Ready Ready Preview Sep 30, 2026 9:41am UTC

@github-actions

Copy link
Copy Markdown

React Doctor found no new issues. 🎉

Reviewed by React Doctor for commit 6d51a53.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: e3d5c0ea-d2fe-4fcc-8215-ae224b38e4b4

📥 Commits

Reviewing files that changed from the base of the PR and between 58dd029 and 6d51a53.

📒 Files selected for processing (2)
  • src/hooks/useNoticeTimer.ts
  • tests/useNoticeTimer.test.tsx

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


Walkthrough

本次修改在 onPause 同步已过时间后清除最后一次 RAF 时间戳。新增测试验证重复暂停及恢复后的剩余计时和关闭回调。

Changes

通知计时器暂停与恢复

Layer / File(s) Summary
暂停时间戳与恢复计时验证
src/hooks/useNoticeTimer.ts, tests/useNoticeTimer.test.tsx
onPause 同步已过时间后将最后一次 RAF 时间戳设为 null。新增测试验证重复暂停期间回调次数不变且未关闭,并确认恢复后按剩余时长计时并触发一次关闭。

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 6d51a

The change prevents repeated pauses from counting time spent hovering while preserving the remaining notice duration after resume. No actionable merge-blocking risk is identified; merge after normal checks pass.

Architecture Summary

Architecture risk: 🔵 Low · up to 6d51a

The change affects 2 systems.

Changed systems: src, tests

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 1 changed file maps to changed impact.
  • observed — tests (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in src/hooks/useNoticeTimer.ts: onPause 在同步经过时间后清除最后一次 RAF 时间戳;此前此处没有该赋值。
  • observed — Modified behavior in tests/useNoticeTimer.test.tsx: 新增 useNoticeTimer 测试,覆盖重复暂停期间的回调断言,以及恢复后按剩余时长推进并最终触发关闭的断言;测试前后启用和还原假定时器,并在结束时卸载 hook。
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确概括了主要变更:重复暂停计时器时保留剩余时长。该描述与代码修改、回归测试和 PR 目标一致,且简洁明确。
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@codecov

codecov Bot commented Sep 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.21%. Comparing base (58dd029) to head (6d51a53).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #417   +/-   ##
=======================================
  Coverage   99.21%   99.21%           
=======================================
  Files          12       12           
  Lines         380      381    +1     
  Branches      103      103           
=======================================
+ Hits          377      378    +1     
  Misses          3        3           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

❌ Deploy failed

PR preview ❌ Failed ❌ Failed
🔗 Preview https://react-component-notification-preview-pr-417.surge.sh (may be unavailable)
📝 Commit6d51a53
🪵 LogsView logs
📋 Build log (last lines)
npm error     @eslint-community/eslint-utils@"^4.9.1" from @typescript-eslint/utils@8.71.0
npm error     node_modules/@typescript-eslint/utils
npm error       @typescript-eslint/utils@"8.71.0" from @typescript-eslint/eslint-plugin@8.71.0
npm error       node_modules/@typescript-eslint/eslint-plugin
npm error         peerOptional @typescript-eslint/eslint-plugin@"^8.0.0" from eslint-plugin-jest@29.16.6
npm error         node_modules/eslint-plugin-jest
npm error         1 more (typescript-eslint)
npm error       3 more (@typescript-eslint/type-utils, eslint-plugin-jest, typescript-eslint)
npm error     @eslint-community/eslint-utils@"^4.8.0" from eslint@10.11.0
npm error   10 more (@eslint/compat, @eslint/js, ...)
npm error
npm error Could not resolve dependency:
npm error peer eslint@"^3 || ^4 || ^5 || ^6 || ^7 || ^8 || ^9.7" from eslint-plugin-react@7.37.5
npm error node_modules/eslint-plugin-react
npm error   dev eslint-plugin-react@"^7.37.5" from the root project
npm error
npm error Conflicting peer dependency: eslint@9.39.5
npm error node_modules/eslint
npm error   peer eslint@"^3 || ^4 || ^5 || ^6 || ^7 || ^8 || ^9.7" from eslint-plugin-react@7.37.5
npm error   node_modules/eslint-plugin-react
npm error     dev eslint-plugin-react@"^7.37.5" from the root project
npm error
npm error Fix the upstream dependency conflict, or retry
npm error this command with --force or --legacy-peer-deps
npm error to accept an incorrect (and potentially broken) dependency resolution.
npm error
npm error
npm error For a full report see:
npm error /home/runner/.npm/_logs/2026-09-30T09_43_03_306Z-eresolve-report.txt
npm error A complete log of this run can be found in: /home/runner/.npm/_logs/2026-09-30T09_43_03_306Z-debug-0.log

🤖 Powered by surge-preview

@zombieJ
zombieJ merged commit ccec0ec into master Sep 30, 2026
15 checks passed
@zombieJ
zombieJ deleted the codex/notification-repeated-pause branch September 30, 2026 09:48

This branch was successfully deployed

1 active deployment
Preview – notification — 6d51a53f Deployed Sep 30, 2026 by vercel[bot]
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.

1 participant