feat(#207): SW notificationclick で trip / block へ deep link 遷移 - #209
Open
kuu13580 wants to merge 6 commits into
Open
feat(#207): SW notificationclick で trip / block へ deep link 遷移#209kuu13580 wants to merge 6 commits into
kuu13580 wants to merge 6 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughFCM通知クリック時に、Service Workerが同一オリジンのクライアントへSPA遷移を通知します。保留URLはCache Storageでも処理します。TripPageは Changes通知ディープリンク
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant User
participant FirebaseMessagingSW
participant CacheStorage
participant App
participant TripPage
participant BlockElement
User->>FirebaseMessagingSW: notificationclick
FirebaseMessagingSW->>CacheStorage: store pending intent
FirebaseMessagingSW->>App: postMessage FCM_NAVIGATE
App->>TripPage: navigate to pathname, search, hash
TripPage->>TripPage: process focusBlock
TripPage->>BlockElement: scrollIntoView center
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
Visit the preview URL for this PR (updated for commit a6638e7): https://tabi-share-8ef6b--pr209-feature-issue207-sw-n7jp3kfl.web.app (expires Sat, 08 Aug 2026 07:36:31 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 9f2a87ede127df7673322845e34cf22c1372d720 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@frontend/public/firebase-messaging-sw.js`:
- Around line 40-53: Move the self.addEventListener('notificationclick', ...)
registration, including its custom deep-link validation and navigation logic,
before the Firebase importScripts, initializeApp, and onBackgroundMessage setup
in firebase-messaging-sw.js. Preserve the existing same-origin checks and click
behavior while ensuring the custom handler is registered first.
In `@frontend/src/hooks/useFocusBlockOnMount.ts`:
- Line 20: Update the consumedRef logic in useFocusBlockOnMount so it stores the
last processed focusBlock value rather than a boolean. Skip processing only when
the current value matches the stored value, then update the ref when handling a
new value while preserving React StrictMode duplicate-effect protection and
ensuring each new notification changes selectedPageId, scrolls, and calls
clearParam().
- Line 1: useFocusBlockOnMount の consumedRef を boolean の消費済みフラグから、直前に処理した
rawFocusBlock 値を保持する ref に変更し、値が変わった場合は再処理して selectedPageId
更新・scrollIntoView・query 掃除を実行できるようにする。同一マウント中に focusBlock=42 から 99
へ連続して変化した場合、2回目の処理結果を検証するテストを useFocusBlockOnMount の既存テストへ追加する。
🪄 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: 56b64829-c350-4706-80b1-a943af8009c8
📒 Files selected for processing (12)
docs/memo/notification_roadmap.mddocs/notifications.mdfrontend/public/firebase-messaging-sw.jsfrontend/src/App.tsxfrontend/src/components/blocks/view/BlockScheduleView.tsxfrontend/src/components/blocks/view/BlockTransportationView.tsxfrontend/src/hooks/useBlocks.tsfrontend/src/hooks/useFcmNavigationListener.test.tsxfrontend/src/hooks/useFcmNavigationListener.tsfrontend/src/hooks/useFocusBlockOnMount.test.tsxfrontend/src/hooks/useFocusBlockOnMount.tsfrontend/src/pages/TripPage.tsx
kuu13580
force-pushed
the
feature/issue207_sw-notificationclick-deeplink
branch
from
July 31, 2026 00:00
6471a2b to
bb41626
Compare
Contributor
Author
|
@coderabbitai 指摘 3 件を fixup で反映しました。
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Service worker outline =='
ast-grep outline frontend/public/firebase-messaging-sw.js --items all || true
echo '== Service worker event/Firebase ordering =='
rg -n -C 5 'notificationclick|importScripts|initializeApp|onBackgroundMessage' frontend/public/firebase-messaging-sw.js
echo '== Focus hook outline =='
ast-grep outline frontend/src/hooks/useFocusBlockOnMount.ts --items all || true
echo '== Focus hook consumption logic =='
rg -n -C 8 'consumed(Key)?Ref|rawFocusBlock|useEffect' frontend/src/hooks/useFocusBlockOnMount.ts
echo '== Regression test evidence =='
rg -n -C 10 '42|99|focusBlock|scrollIntoView|selectedPageId' frontend/src/hooks/useFocusBlockOnMount.test.tsxLength of output: 12777
ご対応ありがとうございます。🐰 If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
通知タップ時の挙動を Firebase デフォルトから明示ハンドラに置き換え、既存
PWA/tab を単一 tab のまま該当 block まで scroll できるようにする。
- SW: notificationclick で clients.matchAll → focused > visible > 任意
の優先度で client を選び、postMessage で SPA navigate。無ければ
clients.openWindow。別 origin URL は payload 経路含め弾く
- client 側: useFcmNavigationListener が SW message を受け React Router
で navigate。WindowClient.navigate() のフルリロードを避けて atom /
SWR / scroll 位置を保持
- useFocusBlockOnMount: ?focusBlock={id} を useBlock で解決 → pageId
切替 → data-block-id の DOM を rAF 待機して scrollIntoView (center)
→ replaceState で query 除去。404 / 非数値ゴミは黙って掃除
- useBlocks: list fetch 時に個別 key を populate し useBlock の重複
fetch を回避 (楽観更新値がある場合は skip して巻き戻し防止)
- View block 2 種に data-block-id を付与 (edit モードは対象外)
- useFcmNavigationListener: FCM_NAVIGATE 受信で navigate 呼び出し / 別 origin & 非 http scheme & 型不正 payload の無視 / unmount cleanup / serviceWorker 未定義環境 - useFocusBlockOnMount: block ロード成功で selectedPageId 切替 + scrollIntoView + query 除去 / 404 → query 除去のみ / 非数値 → API も叩かず query 除去 / focusBlock 無 → no-op
- docs/notifications.md: §5 Deep link を実装済み記述に更新 (SW ハンドラ → SPA navigate → useFocusBlockOnMount → scrollIntoView の経路と、 処理後の replaceState 除去まで明記) - docs/memo/notification_roadmap.md: P6b を完了へ
kuu13580
force-pushed
the
feature/issue207_sw-notificationclick-deeplink
branch
from
August 1, 2026 00:10
bb41626 to
c22d5ce
Compare
kuu13580
marked this pull request as ready for review
August 1, 2026 01:08
kuu13580
requested review from
bobtaroh and
koki-fore
and removed request for
a team
August 1, 2026 01:08
There was a problem hiding this comment.
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 `@frontend/src/hooks/useFocusBlockOnMount.ts`:
- Around line 41-44: Update the useFocusBlockOnMount hook so processing is
tracked per navigation event rather than only by rawFocusBlock, allowing a
second navigation to the same block ID to run page selection, scrolling, and
query removal again. Record the event as consumed only after its uncancelled
processing completes, and add a regression test covering 42 → query removal → 42
within the same mount.
- Around line 33-38: Update the focusBlock parsing in useFocusBlockOnMount so
only positive safe integers are accepted; treat 0, unsafe integers, and other
invalid values as invalid and remove the query parameter. Add a regression test
covering focusBlock=0, and keep useBlock’s existing falsy-key behavior
unchanged.
🪄 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: 1fca4e6e-8b12-4300-80a1-a2793f56f227
📒 Files selected for processing (12)
docs/memo/notification_roadmap.mddocs/notifications.mdfrontend/public/firebase-messaging-sw.jsfrontend/src/App.tsxfrontend/src/components/blocks/view/BlockScheduleView.tsxfrontend/src/components/blocks/view/BlockTransportationView.tsxfrontend/src/hooks/useBlocks.tsfrontend/src/hooks/useFcmNavigationListener.test.tsxfrontend/src/hooks/useFcmNavigationListener.tsfrontend/src/hooks/useFocusBlockOnMount.test.tsxfrontend/src/hooks/useFocusBlockOnMount.tsfrontend/src/pages/TripPage.tsx
🚧 Files skipped from review as they are similar to previous changes (10)
- frontend/src/components/blocks/view/BlockScheduleView.tsx
- docs/notifications.md
- frontend/src/App.tsx
- frontend/src/components/blocks/view/BlockTransportationView.tsx
- docs/memo/notification_roadmap.md
- frontend/src/hooks/useBlocks.ts
- frontend/src/pages/TripPage.tsx
- frontend/src/hooks/useFcmNavigationListener.ts
- frontend/public/firebase-messaging-sw.js
- frontend/src/hooks/useFcmNavigationListener.test.tsx
- focusBlock を正の safe integer のみ許可する parse に変更。regex を /^[1-9]\d*$/ + Number.isSafeInteger + >0 の三重チェックへ。0 が 通ると useBlock(0) が SWR falsy key で fetch を止め ref も query も更新されずスタックしていた - consumedKeyRef のセットを async の uncancelled 完了後に移動。 StrictMode dev double-fire で最初の async が cancel された時に scroll が発火しなくなる問題を解消 - rawFocusBlock === null (query 掃除後) 時点で ref をリセット。同一 block の再通知が同一マウント中に来ても再処理できるようにする - 回帰テスト 2 種追加 (focusBlock=0 / 42 → clearParam → 42 再処理)
Contributor
Author
|
@coderabbitai 2 件の指摘を追加コミット (`bc92db0`) で反映しました。
`vitest 207 passed` (新規 2) / `type-check` / `lint` / `format` 通過。 |
|
Tip For best results, initiate chat on the files or code changes.
ご対応ありがとうございます。 You are interacting with an AI system. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/hooks/useFocusBlockOnMount.test.tsx (1)
220-221: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
useBlocksの重複取得回避を検証してください。Line 220 の MSW handler は同じ応答を複数回返します。リクエスト回数を記録していないため、2回目の
focusBlock=42処理が/blocks/42を再取得しても、このテストは成功します。1回目の処理後の回数を保存し、2回目の処理後も増えないことを検証してください。テスト修正案
+ let requestCount = 0; server.use(http.get('*/blocks/42', () => { + requestCount += 1; return HttpResponse.json(scheduleBlockJson(42, 7)); })); ... expect(scrollIntoViewMock).toHaveBeenCalledTimes(1); + const requestCountAfterFirst = requestCount; ... await waitFor(() => { expect(scrollIntoViewMock).toHaveBeenCalledWith({ behavior: 'smooth', block: 'center' }); }); + expect(requestCount).toBe(requestCountAfterFirst);Also applies to: 254-278
🤖 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 `@frontend/src/hooks/useFocusBlockOnMount.test.tsx` around lines 220 - 221, Update the test around the MSW handler and the two focusBlock=42 flows to count requests to /blocks/42, record the count after the first processing, and assert that the count remains unchanged after the second processing. Preserve the existing response setup while explicitly verifying useBlocks avoids the duplicate fetch.
🤖 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.
Nitpick comments:
In `@frontend/src/hooks/useFocusBlockOnMount.test.tsx`:
- Around line 220-221: Update the test around the MSW handler and the two
focusBlock=42 flows to count requests to /blocks/42, record the count after the
first processing, and assert that the count remains unchanged after the second
processing. Preserve the existing response setup while explicitly verifying
useBlocks avoids the duplicate fetch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 89d43864-3e49-4939-bf9c-23a5b56bf77e
📒 Files selected for processing (2)
frontend/src/hooks/useFocusBlockOnMount.test.tsxfrontend/src/hooks/useFocusBlockOnMount.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/hooks/useFocusBlockOnMount.ts
useBlock(id) 単発 fetch → selectedPageId 切替の後、対象 page の useBlocks(pageId) list fetch が終わるまで ViewTripLayout は Skeleton を 返し、data-block-id DOM がまだ存在しない。Cloud Run コールドスタート等で list fetch が 3 秒を超えると rAF ポーリングが空振りして scroll が 発火しないケースがあった。 - rAF ポーリングを MutationObserver に変更。Skeleton が Timeline に 差し替わった瞬間に即発火するようになり無駄回転をなくす - timeout を 3s → 8s に延長 (staging 実測でも 3s 超えるケースを確認) - 要素発見後に 1 rAF 挟んでから scrollIntoView。Timeline 差し替え直後は layout がまだ確定していないタイミングで scroll してもズレることがあった
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 `@frontend/src/hooks/useFocusBlockOnMount.ts`:
- Around line 22-43: Update the useFocusBlockOnMount flow, including
waitForBlockElement and scrollIntoViewOnNextFrame, to use cancellable cleanup
state that explicitly disconnects the MutationObserver, clears the timeout, and
cancels any pending requestAnimationFrame when focusBlock changes or the
component unmounts. Preserve the 42→99 transition behavior and add regression
coverage for that transition and unmount during waiting.
🪄 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: 859de847-11cb-4e67-a98e-40088cc1ea8d
📒 Files selected for processing (2)
frontend/src/hooks/useFocusBlockOnMount.test.tsxfrontend/src/hooks/useFocusBlockOnMount.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/hooks/useFocusBlockOnMount.test.tsx
Comment on lines
+22
to
+43
| const waitForBlockElement = (blockId: number, maxMs: number, isCancelled: () => boolean): Promise<HTMLElement | null> => | ||
| new Promise(resolve => { | ||
| const selector = `[data-block-id="${blockId}"]`; | ||
|
|
||
| const found = document.querySelector<HTMLElement>(selector); | ||
| if (found) return resolve(found); | ||
|
|
||
| let timeoutId: ReturnType<typeof setTimeout> | null = null; | ||
| const finish = (el: HTMLElement | null) => { | ||
| observer.disconnect(); | ||
| if (timeoutId !== null) clearTimeout(timeoutId); | ||
| resolve(el); | ||
| }; | ||
|
|
||
| const observer = new MutationObserver(() => { | ||
| if (isCancelled()) return finish(null); | ||
| const el = document.querySelector<HTMLElement>(selector); | ||
| if (el) finish(el); | ||
| }); | ||
| observer.observe(document.body, { childList: true, subtree: true }); | ||
| timeoutId = setTimeout(() => finish(null), maxMs); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching hook:"
git ls-files | rg '(^|/)useFocusBlockOnMount\.(ts|tsx|js|jsx)$|useFocusBlockOnMount' || true
file="$(git ls-files | rg '(^|/)useFocusBlockOnMount\.(ts|tsx|js|jsx)$' | head -n 1 || true)"
if [ -n "$file" ]; then
echo
echo "File: $file"
wc -l "$file"
echo
sed -n '1,180p' "$file" | cat -n
fiRepository: tabi-bit/tabi-share
Length of output: 5538
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate tests:"
git ls-files | rg '(^|/)useFocusBlockOnMount\.test\.(tsx|ts|jsx|js)$' || true
test_file="$(git ls-files | rg '(^|/)useFocusBlockOnMount\.test\.(tsx|ts|jsx|js)$' | head -n 1 || true)"
if [ -n "$test_file" ]; then
echo
echo "File: $test_file"
wc -l "$test_file"
echo
sed -n '1,260p' "$test_file" | cat -n
fi
echo
echo "Behavioral probe of current resource lifecycle based on static source:"
python3 - <<'PY'
from pathlib import Path
p = Path('frontend/src/hooks/useFocusBlockOnMount.ts')
s = p.read_text()
checks = {
'cleanup_only_sets_cancelled': 'return () => {\n cancelled = true;\n }' in s,
'observer_disconnected_in_finish': 'observer.disconnect()' in s,
'observer_callback_calls_isCancelled': 'if (isCancelled()) return finish(null)' in s,
'timeout_cleared_in_finish': 'clearTimeout(timeoutId)' in s,
'raf_returns_id': 'const ' in s and 'requestAnimationFrame' in s and 'frameId' in s,
'cleanup_releases_obs_or_timeout_or_raf': any(x in 'return () => {\n cancelled = true;\n }' for x in ['observer.disconnect','clearTimeout','clearAnimationFrame','frameId']),
'has_cancel_handle_param_to_async_path': 'AbortSignal' in s or 'oncancel' in s or 'cancel' in s,
'tests_mention_could_retain_rare': bool((p.parent / f'{p.stem}.test.tsx').read_text() if (p.parent / f'{p.stem}.test.tsx').exists() else ''),
}
# Inspect exact lines by counting before/after.
lines = s.splitlines()
for i, line in enumerate(lines, 1):
if 'function waitForBlockElement' in line or 'const waitForBlockElement' in line:
print('waitForBlockElement starts at line', i)
for n,l in enumerate(lines[max(1,i-5):min(len(lines),i+25)], i-5):
if n <= len(lines)+5:
print(f'{n}: {l}')
break
for k,v in checks.items():
print(f'{k}: {v}')
PYRepository: tabi-bit/tabi-share
Length of output: 11761
cleanup で待機中の observer、timer、rAF を解放してください。
cleanup は cancelled = true までの設定だけで、待機中の MutationObserver と 8 秒タイマーは明示的に解放されません。scrollIntoViewOnNextFrame も requestAnimationFrame の ID を保持しないため、focusBlock が変わると旧ブロックの scroll 処理が残る可能性があります。AbortSignal など cancel 可能なフローへ移し、cleanup で observer、timer、rAF を停止してください。対応後の 42 → 99 遷移・アンマウント中の回帰テストも追加してください。
🤖 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 `@frontend/src/hooks/useFocusBlockOnMount.ts` around lines 22 - 43, Update the
useFocusBlockOnMount flow, including waitForBlockElement and
scrollIntoViewOnNextFrame, to use cancellable cleanup state that explicitly
disconnects the MutationObserver, clears the timeout, and cancels any pending
requestAnimationFrame when focusBlock changes or the component unmounts.
Preserve the 42→99 transition behavior and add regression coverage for that
transition and unmount during waiting.
現状は SW → client の postMessage 1 本に依存しており、以下 2 パターンで navigate が発火せず PWA が前面化したまま URL が変わらない事故になる: - Android で PWA が freeze/kill され、matchAll は phantom client を 返すが event loop で message が drain されない - cold start で client 側 message listener 登録より前に SW が postMessage を送出 Cache Storage を safety net にして両方救う。 - SW: 通知タップの target URL を Cache に put してから postMessage/ openWindow へ進む - Client: useFcmNavigationListener を mount + visibilitychange で Cache 内 intent を消化する経路に拡張。fast path (message) と safety net (cache) の二重発火を lastNavigatedRef で dedupe - 現在 URL と一致するときは navigate せず、履歴汚染を避ける - Cache API 非対応環境 / QuotaExceeded は best effort として無視
kuu13580
force-pushed
the
feature/issue207_sw-notificationclick-deeplink
branch
from
August 1, 2026 07:35
71cb265 to
a6638e7
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
frontend/src/components/DebugLogPanel.tsx (1)
59-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value不要なコメントを削除してください。
Line 59 の説明は、直後の
window.prompt('コピーしてください', logs)と分岐条件から判断できます。コーディングガイドラインに基づき、
**/*.{js,ts,jsx,tsx}では「Add comments in Japanese, but only when the intent cannot be read from the code itself, keeping them to a minimum」です。🤖 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 `@frontend/src/components/DebugLogPanel.tsx` at line 59, DebugLogPanel のクリップボード拒否時のフォールバック処理にある冗長な日本語コメントを削除し、直後の window.prompt 呼び出しと分岐条件だけで意図が伝わる状態にしてください。Source: Coding guidelines
🤖 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 `@frontend/src/components/DebugLogPanel.tsx`:
- Around line 71-75: DebugLogPanel の onCopy および onClear
ボタンに表示しているユーザー向け英語テキストを日本語へ変更し、UI 表記を日本語に統一してください。
In `@frontend/src/hooks/useFcmNavigationListener.ts`:
- Around line 69-100: Update navigateOnce in useFcmNavigationListener so
lastNavigatedRef does not permanently suppress later notifications for the same
target. Clear the reference shortly after navigate(target) completes, or reset
it when the current URL matches the target, while preserving duplicate
suppression for the immediate message/cache delivery.
In `@frontend/src/lib/debugLogger.ts`:
- Around line 108-115: Update frontend/src/lib/debugLogger.ts lines 108-115 in
clearAllLogs to await the IndexedDB transaction’s completion handler and reject
on error or abort, resolving only after clear succeeds; update
frontend/src/components/DebugLogPanel.tsx lines 64-67 in onClear to show the
success toast only when clearAllLogs completes successfully, while preserving
failure handling.
---
Nitpick comments:
In `@frontend/src/components/DebugLogPanel.tsx`:
- Line 59: DebugLogPanel のクリップボード拒否時のフォールバック処理にある冗長な日本語コメントを削除し、直後の
window.prompt 呼び出しと分岐条件だけで意図が伝わる状態にしてください。
🪄 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: dedebb1b-c48e-403b-957a-f2165449e419
📒 Files selected for processing (7)
frontend/public/firebase-messaging-sw.jsfrontend/src/App.tsxfrontend/src/components/DebugLogPanel.tsxfrontend/src/hooks/useFcmNavigationListener.test.tsxfrontend/src/hooks/useFcmNavigationListener.tsfrontend/src/hooks/useFocusBlockOnMount.tsfrontend/src/lib/debugLogger.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/App.tsx
- frontend/src/hooks/useFocusBlockOnMount.ts
Comment on lines
+71
to
+75
| <button type='button' onClick={onCopy} className='cursor-pointer rounded bg-teal-600 px-2 py-1 hover:bg-teal-500'> | ||
| Copy Logs | ||
| </button> | ||
| <button type='button' onClick={onClear} className='cursor-pointer rounded bg-red-600 px-2 py-1 hover:bg-red-500'> | ||
| Clear |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
ボタン表示を日本語にしてください。
Line 72 の Copy Logs と Line 75 の Clear はユーザー向け UI テキストです。日本語表示に統一してください。
修正例
- Copy Logs
+ ログをコピー
...
- Clear
+ ログをクリアコーディングガイドラインに基づき、**/*.{json,md,ts,tsx,js,jsx} の UI テキストは日本語で記述してください。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button type='button' onClick={onCopy} className='cursor-pointer rounded bg-teal-600 px-2 py-1 hover:bg-teal-500'> | |
| Copy Logs | |
| </button> | |
| <button type='button' onClick={onClear} className='cursor-pointer rounded bg-red-600 px-2 py-1 hover:bg-red-500'> | |
| Clear | |
| <button type='button' onClick={onCopy} className='cursor-pointer rounded bg-teal-600 px-2 py-1 hover:bg-teal-500'> | |
| ログをコピー | |
| </button> | |
| <button type='button' onClick={onClear} className='cursor-pointer rounded bg-red-600 px-2 py-1 hover:bg-red-500'> | |
| ログをクリア |
🤖 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 `@frontend/src/components/DebugLogPanel.tsx` around lines 71 - 75,
DebugLogPanel の onCopy および onClear ボタンに表示しているユーザー向け英語テキストを日本語へ変更し、UI
表記を日本語に統一してください。
Source: Coding guidelines
Comment on lines
+69
to
+100
| export const useFcmNavigationListener = () => { | ||
| const navigate = useNavigate(); | ||
| // fast path (message) と safety net (cache) が同一 URL に対して二重発火するのを防ぐ。 | ||
| // 直近の遷移先を保持し、同値なら skip する。 | ||
| const lastNavigatedRef = useRef<string | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| let cancelled = false; | ||
|
|
||
| const navigateOnce = (target: string, source: string) => { | ||
| const current = `${window.location.pathname}${window.location.search}${window.location.hash}`; | ||
| if (target === current) { | ||
| void debugLog('CL', 'navigateOnce skip (== current)', { target, source }); | ||
| return; | ||
| } | ||
| if (lastNavigatedRef.current === target) { | ||
| void debugLog('CL', 'navigateOnce skip (== last)', { target, source }); | ||
| return; | ||
| } | ||
| lastNavigatedRef.current = target; | ||
| void debugLog('CL', 'navigate() called', { target, source }); | ||
| navigate(target); | ||
| }; | ||
|
|
||
| const applyIntent = async () => { | ||
| const url = await readAndClearPendingIntent(); | ||
| void debugLog('CL', 'applyIntent', { url }); | ||
| if (cancelled || !url) return; | ||
| const target = parseSameOriginPath(url); | ||
| if (!target) return; | ||
| navigateOnce(target, 'intent'); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
lastNavigatedRefが永続的に残るため、同一URLへの再通知を処理できません。
navigateOnceはlastNavigatedRef.current === targetのとき常にnavigateをスキップします。この参照はフックのライフタイム全体で保持され、成功したnavigate以外でクリアされません。
以下の手順で再通知が失われます。
- 通知Aで
/trip/xyz?focusBlock=7へnavigateし、lastNavigatedRefに同じ値がセットされる。 - TripPage側でクエリが除去され、現在URLは
/trip/xyzになる(lastNavigatedRefは更新されない)。 - ユーザーが他ページへ移動する。
- 後日、同じblockに対する通知が再送され、同じURL(
/trip/xyz?focusBlock=7)がmessageまたはcache intentとして届く。 target !== currentだがlastNavigatedRef.current === targetが真のため、navigate()が呼ばれない。
PR概要には「query除去後に状態をリセットし、同じblockへの再通知を再処理」という要件がありますが、この重複排除ロジックが本フックの段階でnavigate自体を止めてしまうため、下流のblock選択ロジックまで再通知が到達しません。
navigate()呼び出し後に短時間でlastNavigatedRefをクリアするか、currentとtargetが一致した時点でlastNavigatedRefもリセットしてください。
🐛 修正案
const navigateOnce = (target: string, source: string) => {
const current = `${window.location.pathname}${window.location.search}${window.location.hash}`;
if (target === current) {
void debugLog('CL', 'navigateOnce skip (== current)', { target, source });
+ // 現在地と一致した時点で dedupe 参照もクリアし、将来の同一URLへの再通知を処理可能にする
+ lastNavigatedRef.current = null;
return;
}
if (lastNavigatedRef.current === target) {
void debugLog('CL', 'navigateOnce skip (== last)', { target, source });
return;
}
lastNavigatedRef.current = target;
void debugLog('CL', 'navigate() called', { target, source });
navigate(target);
};🤖 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 `@frontend/src/hooks/useFcmNavigationListener.ts` around lines 69 - 100, Update
navigateOnce in useFcmNavigationListener so lastNavigatedRef does not
permanently suppress later notifications for the same target. Clear the
reference shortly after navigate(target) completes, or reset it when the current
URL matches the target, while preserving duplicate suppression for the immediate
message/cache delivery.
Comment on lines
+108
to
+115
| export const clearAllLogs = async (): Promise<void> => { | ||
| try { | ||
| const db = await getDb(); | ||
| const tx = db.transaction(STORE_NAME, 'readwrite'); | ||
| tx.objectStore(STORE_NAME).clear(); | ||
| } catch { | ||
| // ignore | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Locate files:\n'
git ls-files | rg 'frontend/src/(lib/debugLogger\.ts|components/DebugLogPanel\.tsx)$|frontend/src/lib/debugLogger|frontend/src/components/DebugLogPanel' || true
if [ -f frontend/src/lib/debugLogger.ts ]; then
printf '\nFile outline:\n'
ast-grep outline frontend/src/lib/debugLogger.ts || true
printf '\nRelevant debugLogger.ts lines:\n'
sed -n '1,160p' frontend/src/lib/debugLogger.ts | nl -ba
fi
if [ -f frontend/src/components/DebugLogPanel.tsx ]; then
printf '\nRelevant DebugLogPanel.tsx lines:\n'
sed -n '1,110p' frontend/src/components/DebugLogPanel.tsx | nl -ba
fi
printf '\nSearch clearAllLogs usages:\n'
rg -n "clearAllLogs|toast\.success\(|toast\.error|onClear" frontend/src || true
# Deterministic/semantic behavior probe: parse the function and check whether any await/completion handlers occur after transaction start or whether transaction abort/error paths are reachable from clear().
python3 - <<'PY'
from pathlib import Path
p = Path('frontend/src/lib/debugLogger.ts')
if p.exists():
text = p.read_text()
start = text.find('export const clearAllLogs')
end = text.find('\n};', start)
body = text[start:end+3] if end != -1 else ''
print('clearAllLogs contains clear():', 'clear()' in body)
print('clearAllLogs contains tx.await/oncomplete/onabort/onerror wait:', any(s in body for s in ['oncomplete', 'onabort', 'onerror', 'addEventListener'])))
print('clearAllLogs catches:', body.find('catch') != -1)
print('clearAllLogs returns boolean-like indicators:', bool(set(('true', 'false')).intersection(body.split())) )
PYRepository: tabi-bit/tabi-share
Length of output: 928
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Relevant debugLogger.ts lines:\n'
sed -n '1,150p' frontend/src/lib/debugLogger.ts | awk '{ printf "%4d\t%s\n", NR, $0 }'
printf '\nRelevant DebugLogPanel.tsx lines:\n'
sed -n '1,100p' frontend/src/components/DebugLogPanel.tsx | awk '{ printf "%4d\t%s\n", NR, $0 }'
printf '\nUsages:\n'
rg -n 'clearAllLogs|clear\(|transaction\(|onComplete|oncomplete|onabort|onerror|toast\.success|toast\.error' frontend/src || true
printf '\nStatic check of clearAllLogs body:\n'
python3 - <<'PY'
from pathlib import Path
text = Path('frontend/src/lib/debugLogger.ts').read_text()
start = text.find('export const清')
print('contains clearAllLogs:', 'export const clearAllLogs' in text)
# simple extraction until next exported const/function at same indentation is hard in TS, so just print signature/body markers
for marker in ['export const clearAllLogs', 'getDb()', 'db.transaction', 'objectStore(STORE_NAME).clear()', 'oncomplete', 'onabort', 'onerror', 'catch {']:
print(f'{marker}: {marker in text}')
PYRepository: tabi-bit/tabi-share
Length of output: 10464
削除完了を確認してから成功を表示してください。
clearAllLogs は clear() を開始した後、トランザクションの完了結果を待たずに Promise<void> を解決します。IndexedDB が失敗・abort した場合でも DebugLogPanel の onClear は成功トーストを表示します。clearAllLogs の完了ハンドラまで待機し、成功時だけ成功トーストを表示してください。
📍 Affects 2 files
frontend/src/lib/debugLogger.ts#L108-L115(this comment)frontend/src/components/DebugLogPanel.tsx#L64-L67
🤖 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 `@frontend/src/lib/debugLogger.ts` around lines 108 - 115, Update
frontend/src/lib/debugLogger.ts lines 108-115 in clearAllLogs to await the
IndexedDB transaction’s completion handler and reject on error or abort,
resolving only after clear succeeds; update
frontend/src/components/DebugLogPanel.tsx lines 64-67 in onClear to show the
success toast only when clearAllLogs completes successfully, while preserving
failure handling.
kuu13580
added a commit
that referenced
this pull request
Aug 1, 2026
実機ログで SW ハンドラが全く発火せず Firebase SDK デフォルト click 処理 だけが動く現象が発生していたが、原因は VitePWA が firebase-messaging-sw.js を precache していたことにあった。register 時に Chrome が cache 経由で 古い版を fetch → 古い SW が active → いくら deploy しても新版に切り替 わらない。 - vite.config.ts の workbox に globIgnores + navigateFallbackDenylist で firebase-messaging-sw.js を除外 - firebase.json で /firebase-messaging-sw.js に Cache-Control: no-cache ヘッダを追加。Firebase Hosting のデフォルト max-age=3600 も抑える - 診断: DEBUG_LOG_VERSION を v03 に bump、client 側で getRegistrations() を呼び出して全 SW の scope / active / waiting / installing の scriptURL をログ出力 feature branch (#209) に同等の修正を反映する前に、まず diagnostic branch で実機挙動確認する。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #207
Summary
FCM 通知タップ時の挙動を Firebase デフォルトから明示ハンドラに置き換え、既存 PWA/tab を単一 tab のまま該当 block まで自動 scroll するようにする。#148 の P6b (別 issue 送り) の消化。
やったこと
firebase-messaging-sw.js):notificationclickhandler を追加。既存 client をfocused > visible > 任意の優先度で選び、postMessageで SPA navigate +focus()。無ければclients.openWindow。別 origin URL は payload 経路含め弾くuseFcmNavigationListener): SW からのFCM_NAVIGATEmessage を受けて React Router のnavigate()を呼ぶ。App.tsx で 1 回登録useFocusBlockOnMount:?focusBlock={id}をuseBlock(id)で解決 →selectedPageId切替 →data-block-idの DOM を rAF 待機 →scrollIntoView({ behavior: 'smooth', block: 'center' })→replaceStateで query 除去。404 / 非数値ゴミは黙って掃除useBlockspopulate: list fetch 時に個別 key/blocks/{id}に populate しuseBlockの重複 fetch を回避data-block-id付与 (edit モードは対象外)設計上の判断 (コードから読めない部分)
postMessage→focus()を選択 (navigate()ではなく)WindowClient.navigate(url)はブラウザレベルのフル遷移になり atom / SWR / scroll 位置を全て失う。postMessageで client に指示を送って React Router で navigate すれば、同 trip 内の block 遷移は state を保ったまま?focusBlock=の差し替えだけで済み、別 trip でも SPA 遷移で初期 fetch キャッシュを保持できる。client 選択は
focused > visible > 任意に統一PWA client を自己申告方式で明示的に優先する案 (client → SW への postMessage + IndexedDB 永続化) も検討したが、SW ライフサイクル管理のコストに対して「PWA / ブラウザ tab 両方 background + PWA 期待」という限定的なエッジしか救えないため MVP は見送り。iOS は PWA のみで判別問題不発生。
useBlockspopulate は既存個別キャッシュを尊重list revalidation が edit モードの 5s interval で走ると、
useUpdateBlockの楽観更新値をサーバ古い値で上書きする race が発生する。cache.get(key)?.data === undefinedの時のみ populate することで巻き戻し防止。以降の同期はuseUpdateBlock経由で行う。View mode のみ対応 (edit は対象外)
通知タップの用途は閲覧目的が大半で、
data-block-idは View 系 Block コンポーネントにのみ付与。編集は遷移後にユーザが自発的に入れば十分。Test plan
Summary by CodeRabbit
新機能
ドキュメント