Merged
Conversation
FlatListにmaintainVisibleContentPositionを設定し、 リスト先頭にアイテムが追加された際にスクロール位置を自動調整する。 https://claude.ai/code/session_01MPkqpw4PNwHcRH6Gb74HD9
検索、フィルター、件数表示をFlatListの外に移動し、 スクロールしても常に画面上部に固定表示されるようにした。 ログ画面にもmaintainVisibleContentPositionを追加。 https://claude.ai/code/session_01MPkqpw4PNwHcRH6Gb74HD9
|
Important Review skippedThis PR was authored by the user configured for CodeRabbit reviews. By default, CodeRabbit skips reviewing PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback. To trigger a single review, invoke the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughログ／タイムライン画面のヘッダーを固定する stickyHeader に移行し、FlatList に maintainVisibleContentPosition を追加。ADD_LOG で payload.id が無い場合にモジュール内カウンタで Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`(tabs)/logs.tsx:
- Around line 541-544: maintainVisibleContentPosition is being used but keys are
unstable because keyExtractor falls back to index in "item.id ||
`log-${item.timestamp}-${index}`"; ensure stable keys by making LogData.id
mandatory at render time and removing the index-based fallback: validate or
normalize incoming logs (eg. in ADD_LOG flow in lib/location-store.tsx or the
WebSocket handler) to always populate a stable string id before they reach the
FlatList, and update the key extraction logic (the expression referenced above)
to rely solely on item.id so keys don't change when prepending logs.
ADD_LOG reducerでidが空の場合にカウンタベースのIDを生成するようにし、 keyExtractorをitem.idのみに依存させた。 maintainVisibleContentPositionとの組み合わせで 先頭にログが追加されてもキーが変わらないようにした。 https://claude.ai/code/session_01MPkqpw4PNwHcRH6Gb74HD9
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/location-store.tsx (1)
85-89:⚠️ Potential issue | 🟡 Minorテストファイルの reducer と messageCount の動作が異なります。
本番コードでは ADD_LOG 時に
messageCountをインクリメントしていますが（88行目）、テストファイルの reducer ではインクリメントしていません。この不整合により、messageCount 関連のバグがテストで検出されない可能性があります。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/location-store.tsx` around lines 85 - 89, The ADD_LOG handling in the reducer increments messageCount in location-store.tsx (returns messageCount: state.messageCount + 1) but the test reducer does not, causing inconsistent behavior; update the test reducer (the reducer used in your tests) so that its ADD_LOG branch also increments messageCount (or alternatively change the production reducer to match tests if that was intended), ensuring the ADD_LOG case in the reducer, messageCount, and any test fixtures/assertions are aligned with the implementation.
🧹 Nitpick comments (2)
lib/__tests__/location-store.test.ts (2)
234-243: ID が存在しない場合の ADD_LOG テストがありません。PR の目的である「安定した ID の保証」を検証するため、
idが無いペイロードで ADD_LOG をディスパッチした際に ID が自動生成されることをテストすることを推奨します。📝 テスト追加の提案
describe("ADD_LOG", () => { + it("should generate id when payload lacks id", () => { + const logWithoutId = { ...createMockLog(), id: undefined } as unknown as LogData; + const action: LocationAction = { type: "ADD_LOG", payload: logWithoutId }; + + const newState = locationReducer(initialState, action); + + expect(newState.logs[0].id).toBeDefined(); + expect(newState.logs[0].id).toMatch(/^log-gen-/); + }); + it("should add a new log to the beginning of the list", () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/__tests__/location-store.test.ts` around lines 234 - 243, Add a test to verify ADD_LOG generates an ID when payload.id is missing: dispatch an action with type "ADD_LOG" using a log object from createMockLog but with the id property removed or undefined, call locationReducer(initialState, action), then assert the resulting state.logs has length 1 and that state.logs[0].id is defined/non-empty and other fields match the payload; reference createMockLog, locationReducer, initialState, and the LocationAction { type: "ADD_LOG" } shape to locate where to add this test.
52-54: テスト用 reducer と本番 reducer の ID 生成ロジックが異なります。テストでは
log-gen-test-${Date.now()}を使用していますが、本番のlib/location-store.tsxではlog-gen-${++logIdCounter}を使用しています。テストの目的（ID が無い場合に生成される）は達成されていますが、本番コードをインポートしてテストする方が、reducer ロジックの重複を避け、本番コードの実際の動作をテストできます。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/__tests__/location-store.test.ts` around lines 52 - 54, The test's reducer generates IDs with `log-gen-test-${Date.now()}` which diverges from production's `log-gen-${++logIdCounter}`; update the test to reuse the production ID generation instead of rolling its own: import the real reducer or the exported ID helper from lib/location-store.tsx (the `logIdCounter`/generator that produces the `log-gen-` prefix) and replace the inline generation in the test (the `log` assignment that checks `action.payload.id`) so the test calls the production generator when `action.payload.id` is missing, ensuring identical ID semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`(tabs)/logs.tsx:
- Around line 539-542: The maintainVisibleContentPosition prop is not supported
on web (react-native-web ~0.21.2) so implement a web-specific fallback: detect
web (Platform.OS === 'web' or typeof document !== 'undefined') around the
FlatList/ScrollView that currently uses maintainVisibleContentPosition and for
web attach a ref (e.g., listRef), track previous content height via
onContentSizeChange/onLayout, and when items are prepended compute the delta and
call listRef.scrollToOffset/scrollTo to preserve visual position; update the
component in app/(tabs)/logs.tsx to use maintainVisibleContentPosition for
native and the manual measurement + scroll adjustment handlers for web
(reference symbols: maintainVisibleContentPosition, FlatList/ScrollView,
listRef, onContentSizeChange, scrollToOffset).
---
Outside diff comments:
In `@lib/location-store.tsx`:
- Around line 85-89: The ADD_LOG handling in the reducer increments messageCount
in location-store.tsx (returns messageCount: state.messageCount + 1) but the
test reducer does not, causing inconsistent behavior; update the test reducer
(the reducer used in your tests) so that its ADD_LOG branch also increments
messageCount (or alternatively change the production reducer to match tests if
that was intended), ensuring the ADD_LOG case in the reducer, messageCount, and
any test fixtures/assertions are aligned with the implementation.
---
Nitpick comments:
In `@lib/__tests__/location-store.test.ts`:
- Around line 234-243: Add a test to verify ADD_LOG generates an ID when
payload.id is missing: dispatch an action with type "ADD_LOG" using a log object
from createMockLog but with the id property removed or undefined, call
locationReducer(initialState, action), then assert the resulting state.logs has
length 1 and that state.logs[0].id is defined/non-empty and other fields match
the payload; reference createMockLog, locationReducer, initialState, and the
LocationAction { type: "ADD_LOG" } shape to locate where to add this test.
- Around line 52-54: The test's reducer generates IDs with
`log-gen-test-${Date.now()}` which diverges from production's
`log-gen-${++logIdCounter}`; update the test to reuse the production ID
generation instead of rolling its own: import the real reducer or the exported
ID helper from lib/location-store.tsx (the `logIdCounter`/generator that
produces the `log-gen-` prefix) and replace the inline generation in the test
(the `log` assignment that checks `action.payload.id`) so the test calls the
production generator when `action.payload.id` is missing, ensuring identical ID
semantics.
- ADD_LOGケースにmessageCountインクリメントを追加（プロダクションと一致） - ID生成をDate.now()からカウンタベースに変更（プロダクションと同一ロジック） - payload.idが空の場合にIDが生成されることを検証するテストを追加 https://claude.ai/code/session_01MPkqpw4PNwHcRH6Gb74HD9
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/__tests__/location-store.test.ts (2)
254-261:messageCountの回帰防止アサーションも追加したいです。Line 64 の仕様変更をこのテストで直接担保すると、将来のリグレッション検知が強くなります。
差分案
expect(newState.logs).toHaveLength(1); expect(newState.logs[0].id).toBeTruthy(); expect(newState.logs[0].id).toMatch(/^log-gen-/); expect(newState.logs[0].device).toBe(log.device); expect(newState.logs[0].log.message).toBe(log.log.message); + expect(newState.messageCount).toBe(1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/__tests__/location-store.test.ts` around lines 254 - 261, Add an assertion to guard against regressions of the messageCount behavior after calling locationReducer with the action: ensure the state-level counter and the new log entry reflect the expected increment by asserting something like newState.messageCount equals initialState.messageCount + 1 and that newState.logs[0].log.messageCount (or the appropriate field on the log entry) equals the expected value; locate this next to the existing assertions around locationReducer, initialState, action and newState.logs to make the test fail if messageCount changes.
15-16:logIdCounterはテストごとに初期化しておくと安全です。モジュールスコープの可変状態が残るため、将来的に連番を厳密検証するテスト追加時に順序依存になりやすいです。各テストケース開始時に
logIdCounter = 0を入れておくのを推奨します。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/__tests__/location-store.test.ts` around lines 15 - 16, Reset the module-scoped counter logIdCounter at the start of each test to avoid cross-test leakage; add a beforeEach hook in lib/__tests__/location-store.test.ts that sets logIdCounter = 0 (or otherwise reinitialize the counter) so tests that assert sequential IDs remain order-independent and deterministic when referencing the logIdCounter variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/__tests__/location-store.test.ts`:
- Around line 248-252: The test "should generate a stable ID when payload.id is
missing" currently constructs logWithoutId with id set to an empty string, which
doesn't match the "missing" case; update the test so the payload truly omits the
id field (e.g., remove the id property or set it to undefined) instead of
setting it to "" so that the ADD_LOG handling in the reducer/handler (referenced
by LocationAction type and the logWithoutId variable) exercises the missing-id
code path described by the test name.
---
Nitpick comments:
In `@lib/__tests__/location-store.test.ts`:
- Around line 254-261: Add an assertion to guard against regressions of the
messageCount behavior after calling locationReducer with the action: ensure the
state-level counter and the new log entry reflect the expected increment by
asserting something like newState.messageCount equals initialState.messageCount
+ 1 and that newState.logs[0].log.messageCount (or the appropriate field on the
log entry) equals the expected value; locate this next to the existing
assertions around locationReducer, initialState, action and newState.logs to
make the test fail if messageCount changes.
- Around line 15-16: Reset the module-scoped counter logIdCounter at the start
of each test to avoid cross-test leakage; add a beforeEach hook in
lib/__tests__/location-store.test.ts that sets logIdCounter = 0 (or otherwise
reinitialize the counter) so tests that assert sequential IDs remain
order-independent and deterministic when referencing the logIdCounter variable.
- payload.idをundefined（プロパティ除去）に変更し、実際のサーバー欠落ケースに合わせた - messageCountのアサーションを追加してリグレッション防止 - beforeEachでlogIdCounterをリセットしテスト間の状態漏れを防止 https://claude.ai/code/session_01MPkqpw4PNwHcRH6Gb74HD9
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/__tests__/location-store.test.ts (1)
242-250:messageCountのアサーションを追加することを検討してください。
ADD_LOGがmessageCountをインクリメントするようになりましたが、このテストでは検証されていません。新しいテスト（252-269行目）では検証されていますが、基本的な追加テストでも確認すると網羅性が向上します。♻️ 提案
const newState = locationReducer(initialState, action); expect(newState.logs).toHaveLength(1); expect(newState.logs[0]).toEqual(log); + expect(newState.messageCount).toBe(1); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/__tests__/location-store.test.ts` around lines 242 - 250, The test for ADD_LOG currently verifies logs array but not messageCount; update the "should add a new log to the beginning of the list" test to also assert that messageCount was incremented (use locationReducer with initialState and the action and then expect(newState.messageCount).toBe(initialState.messageCount + 1)). Locate symbols: locationReducer, initialState, LocationAction with type "ADD_LOG", and the messageCount property to add the assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/__tests__/location-store.test.ts`:
- Around line 242-250: The test for ADD_LOG currently verifies logs array but
not messageCount; update the "should add a new log to the beginning of the list"
test to also assert that messageCount was incremented (use locationReducer with
initialState and the action and then
expect(newState.messageCount).toBe(initialState.messageCount + 1)). Locate
symbols: locationReducer, initialState, LocationAction with type "ADD_LOG", and
the messageCount property to add the assertion.
The "should add a new log to the beginning of the list" test verified the logs array but not that messageCount was incremented. Add the missing assertion to match the reducer behavior. https://claude.ai/code/session_01MPkqpw4PNwHcRH6Gb74HD9
|
Note Docstrings generation - SKIPPED |
coderabbitai bot
added a commit
that referenced
this pull request
Mar 2, 2026
Docstrings generation was requested by @TinyKitten. * #30 (comment) The following files were modified: * `app/(tabs)/logs.tsx` * `app/(tabs)/timeline.tsx` * `lib/location-store.tsx`
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Member
Author
|
@coderabbitai 今レビュー中ですか？ |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1384
このPRのレビューは既に完了しています（最後のレビューは11:26に投稿されました）。12:16にレビューをリクエストいただきましたが、CodeRabbitは増分レビューシステムのため、既にレビュー済みのコミットは再レビューしません。 新しいコミットがプッシュされた場合は、自動的に差分レビューが実行されます。全体を再レビューしてほしい場合は、 何か特定のファイルやコードについて質問がありましたら、お気軽にお声がけください！ 🧠 Learnings used |
Member
Author
@coderabbitai 指摘があるなら教えてください。指摘がもうないなら approve してください, |
Member
Author
|
@coderabbitai 今どういう状況ですか？ |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1473
まだ 2件の未解決のレビューコメントがあります：
現在のレビューステータスは CHANGES_REQUESTED となっています。 これらのコメントに対応いただくか、または対応不要と判断された場合は「done」や「対応不要」とコメントいただければ、その後 approve させていただきます。 各コメントの詳細は、PRのレビューコメント欄でご確認いただけます。 🧠 Learnings used |
react-native-web ~0.21.2ではmaintainVisibleContentPositionが 未サポートのため、web環境ではonScroll/onContentSizeChangeで スクロール位置を手動補正する方式に切り替える。 ネイティブ環境では従来通りmaintainVisibleContentPositionを使用。 https://claude.ai/code/session_01MPkqpw4PNwHcRH6Gb74HD9
Member
Author
指摘では「id が空文字列 "" に設定されている」とありましたが、現在のコード (line 256) は: const { id: _removed, ...logWithoutId } = log; destructuringで id プロパティを完全に除去しています。logWithoutId.id は undefined（falsy）となり、reducer の action.payload.id ? ... : ... 分岐で正しくID生成パスが実行されます。テストは意図通り動作しており、修正不要です。 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 573
ご説明の通り、コードは： const { id: _removed, ...logWithoutId } = log;destructuring で こちらのレビューコメントは撤回いたします。 残るレビューコメントは (_/) 🧠 Learnings used |
Member
Author
これ対応できている認識だったんですが、できてないんですか？ |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/(tabs)/logs.tsx (1)
252-266: Webのスクロール補正条件を「先頭追加時のみ」に絞ることを推奨します。Line 255-257 は「高さが増えた」場合に常に補正するため、検索条件変更やフィルター解除で件数が増えたときにも補正が走り、意図しないジャンプが起きる可能性があります。先頭 prepend を検知した場合のみ補正する条件にすると安定します。
差分案
-import { useState, useMemo, useCallback, useRef } from "react"; +import { useState, useMemo, useCallback, useRef, useEffect } from "react"; ... - const webScrollState = useRef({ offset: 0, contentHeight: 0 }); + const webScrollState = useRef({ + offset: 0, + contentHeight: 0, + prevFirstId: null as string | null, + prevLength: 0, + shouldCompensate: false, + }); + + useEffect(() => { + if (!isWeb) return; + const s = webScrollState.current; + const nextFirstId = filteredLogs[0]?.id ?? null; + const isPrepend = + s.prevLength > 0 && + filteredLogs.length > s.prevLength && + s.prevFirstId !== null && + nextFirstId !== s.prevFirstId && + filteredLogs.some((x) => x.id === s.prevFirstId); + + s.shouldCompensate = isPrepend; + s.prevFirstId = nextFirstId; + s.prevLength = filteredLogs.length; + }, [isWeb, filteredLogs]); ... const handleWebContentSizeChange = useCallback( (_w: number, h: number) => { const prev = webScrollState.current; - if (prev.contentHeight > 0 && prev.offset > 0) { + if (prev.shouldCompensate && prev.contentHeight > 0 && prev.offset > 0) { const delta = h - prev.contentHeight; if (delta > 0) { listRef.current?.scrollToOffset({ offset: prev.offset + delta, animated: false, }); prev.offset += delta; } } + prev.shouldCompensate = false; prev.contentHeight = h; }, [] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(tabs)/logs.tsx around lines 252 - 266, The handler handleWebContentSizeChange currently adjusts scroll whenever contentHeight increases; restrict this to only when new items were prepended by checking a prepend flag on webScrollState (e.g., webScrollState.current.prepended or webScrollState.current.isPrepend) before applying the delta and updating prev.offset; ensure the code that performs a prepend sets that flag, and clear/reset the flag immediately after performing the scroll correction so future height increases (e.g., from filter/search) won't trigger a jump; continue using listRef.current?.scrollToOffset and update prev.contentHeight as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/`(tabs)/logs.tsx:
- Around line 252-266: The handler handleWebContentSizeChange currently adjusts
scroll whenever contentHeight increases; restrict this to only when new items
were prepended by checking a prepend flag on webScrollState (e.g.,
webScrollState.current.prepended or webScrollState.current.isPrepend) before
applying the delta and updating prev.offset; ensure the code that performs a
prepend sets that flag, and clear/reset the flag immediately after performing
the scroll correction so future height increases (e.g., from filter/search)
won't trigger a jump; continue using listRef.current?.scrollToOffset and update
prev.contentHeight as before.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 219 🏁 Script executed: Length of output: 2345 🏁 Script executed: Length of output: 1686 🏁 Script executed: Length of output: 5314
ご指摘の通り、対応済みです。レビューコメントには「✅ Addressed in commit 9d1a810」と記載されており、現在のコードを確認したところ、
全ての指摘事項が解決済みですので、approve させていただきます。混乱を招いてしまい、申し訳ございませんでした。 (_/) [approve] 🧠 Learnings used |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1890 🏁 Script executed: Length of output: 9860
ご指摘の通り、すでに対応済みです。レビューコメントの本文にも「✅ Addressed in commit 9d1a810」と記載されており、コードを確認したところ： app/(tabs)/logs.tsx では：
という適切な実装がされています。 すべてのレビューコメントが解決済みですので、このまま approve させていただきます。混乱を招いてしまい申し訳ございませんでした。 [approve] (_/) 🧠 Learnings used |
The handleWebContentSizeChange handler was adjusting scroll position on any content height increase, which would cause unwanted jumps when filters reveal more items. Now tracks the first item ID to detect actual prepends and gates scroll correction behind an isPrepend flag that is set on prepend detection and cleared after correction. https://claude.ai/code/session_01MPkqpw4PNwHcRH6Gb74HD9
This was referenced Mar 3, 2026
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
2 participants
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.
Summary
maintainVisibleContentPositionを追加し、スクロール中に新しいデータが先頭に追加されてもスクロール位置を維持するように修正Test plan
https://claude.ai/code/session_01MPkqpw4PNwHcRH6Gb74HD9
Summary by CodeRabbit