画面長押しで開く設定モーダルからテーマを選べるテーマ一覧モーダルを追加#5933
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthrough新しい ThemeListModal を追加し、設定モーダルにテーマ行を追加、親コンポーネントで選択を AsyncStorage に永続化して状態を更新します。 ChangesTheme Selection Modal Integration
🎯 3 (Moderate) | ⏱️ ~20 minutes sequenceDiagram
participant User
participant SelectModal as SelectBoundSettingListModal
participant Parent as PermittedLayout
participant ThemeModal as ThemeListModal
participant Storage as AsyncStorage
participant Atom as themePreferenceAtom
User->>SelectModal: tap "テーマ" ボタン
SelectModal->>Parent: onThemePress()
Parent->>SelectModal: close settings modal (start animation)
Parent->>ThemeModal: open (after close animation)
User->>ThemeModal: select theme (value)
ThemeModal->>Parent: onSelect(theme)
Parent->>Storage: setItem(THEME_PREFERENCE, theme)
Storage-->>Parent: success / error
Parent->>Atom: update themePreference
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/SelectBoundSettingListModal.test.tsx (1)
116-158: ⚡ Quick win起点のボタンだけでなく、今回追加したモーダル遷移フローも1本は保護したいです。
ここで増えたのはテーマ行の表示確認までで、PR の主機能である
onThemePress -> 設定モーダル close 完了 -> ThemeListModal 表示とonSelect -> 永続化が未テストです。PermittedかThemeListModalの sibling test を1本追加しておくと、この変更の回帰耐性がかなり上がります。As per coding guidelines,
**/*.{test,spec}.{ts,tsx,js,jsx,py,java,go,cs}: Test agents in isolation and with integration tests to ensure correctness and compatibility.🤖 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/components/SelectBoundSettingListModal.test.tsx` around lines 116 - 158, Add an integration test to cover the full theme-flow: render SelectBoundSettingListModal (use defaultProps), provide an onThemePress handler that triggers the modal transition, then simulate pressing the theme button (call getByText('theme') and fireEvent.press) and assert that the parent modal was closed (e.g., verify it's no longer in the tree or that a close callback was invoked) and that ThemeListModal (or its Permitted sibling) is rendered; also simulate selecting a theme in ThemeListModal and assert the onSelect persistence callback was called with the chosen value. Locate the flow around the SelectBoundSettingListModal component, the onThemePress prop, ThemeListModal/Permitted rendering, and the onSelect handler to implement this test.
🤖 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/components/ThemeListModal.tsx`:
- Around line 185-191: The FlatList in ThemeListModal is allowed to grow by
content height and pushes the footer off-screen on small devices; constrain it
to the remaining modal space so it scrolls instead. Wrap the FlatList (or apply
directly to the FlatList) with a container/style that uses flex: 1 (e.g., set
style={{flex: 1}} on the FlatList or its parent View) so the list fills only the
available modal height and becomes scrollable; keep renderItem/keyExtractor the
same and preserve contentContainerStyle for item spacing.
---
Nitpick comments:
In `@src/components/SelectBoundSettingListModal.test.tsx`:
- Around line 116-158: Add an integration test to cover the full theme-flow:
render SelectBoundSettingListModal (use defaultProps), provide an onThemePress
handler that triggers the modal transition, then simulate pressing the theme
button (call getByText('theme') and fireEvent.press) and assert that the parent
modal was closed (e.g., verify it's no longer in the tree or that a close
callback was invoked) and that ThemeListModal (or its Permitted sibling) is
rendered; also simulate selecting a theme in ThemeListModal and assert the
onSelect persistence callback was called with the chosen value. Locate the flow
around the SelectBoundSettingListModal component, the onThemePress prop,
ThemeListModal/Permitted rendering, and the onSelect handler to implement this
test.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d9b78409-f780-47f9-91d2-1e11169b266b
📒 Files selected for processing (4)
src/components/Permitted.tsxsrc/components/SelectBoundSettingListModal.test.tsxsrc/components/SelectBoundSettingListModal.tsxsrc/components/ThemeListModal.tsx
Contributor
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/components/ThemeListModal.test.tsx`:
- Around line 45-63: The tests for ThemeListModal are too loose: replace the
hardcoded label checks and the loose getAllByText('select').length > 1 assertion
with deterministic checks based on getSettingsThemes(); render <ThemeListModal
{...defaultProps} /> (and once with
currentPreference=THEME_PREFERENCE.TOKYO_METRO), then assert that every theme
key from getSettingsThemes() is present via getByText, assert
getAllByText('inUse') has length 1, and assert getAllByText('select') has length
equal to getSettingsThemes().length - 1 so the tests fail if themes are
added/removed or rendering changes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08f0406d-50b1-4b97-96b6-151870e7021a
📒 Files selected for processing (2)
src/components/ThemeListModal.test.tsxsrc/components/ThemeListModal.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/ThemeListModal.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/ThemeListModal.test.tsx (1)
66-72: ⚡ Quick win押下対象ラベルのハードコードを避けるとテスト耐性が上がります
Line 71 の
'tokyoMetroLike'固定は、ラベルキー変更だけでテストが壊れます。THEME_PREFERENCE.TOKYO_METROからラベルを引いて押下対象を決める方が安全です。差分案
it('テーマを押すと onSelect がそのテーマで呼ばれる', () => { const onSelect = jest.fn(); + const metroTheme = getSettingsThemes().find( + (theme) => theme.preference === THEME_PREFERENCE.TOKYO_METRO, + ); + if (!metroTheme) { + throw new Error('TOKYO_METRO theme not found'); + } const { getByText } = render( <ThemeListModal {...defaultProps} onSelect={onSelect} /> ); - fireEvent.press(getByText('tokyoMetroLike')); + fireEvent.press(getByText(metroTheme.label)); expect(onSelect).toHaveBeenCalledTimes(1); expect(onSelect).toHaveBeenCalledWith(THEME_PREFERENCE.TOKYO_METRO); });🤖 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/components/ThemeListModal.test.tsx` around lines 66 - 72, The test in ThemeListModal.test.tsx hardcodes the label 'tokyoMetroLike' when pressing the theme; instead derive the label from the source of truth to avoid brittleness. Update the test to compute the target text from THEME_PREFERENCE.TOKYO_METRO (or the same label resolver/mapping your app uses) and then call fireEvent.press(getByText(...)) with that computed label, keeping the test's usage of ThemeListModal and the onSelect jest.fn() intact.
🤖 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 `@src/components/ThemeListModal.test.tsx`:
- Around line 66-72: The test in ThemeListModal.test.tsx hardcodes the label
'tokyoMetroLike' when pressing the theme; instead derive the label from the
source of truth to avoid brittleness. Update the test to compute the target text
from THEME_PREFERENCE.TOKYO_METRO (or the same label resolver/mapping your app
uses) and then call fireEvent.press(getByText(...)) with that computed label,
keeping the test's usage of ThemeListModal and the onSelect jest.fn() intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 44f2e3c5-cb29-4329-8bf5-0659ded600f9
📒 Files selected for processing (2)
src/components/ThemeListModal.test.tsxsrc/components/ThemeListModal.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/ThemeListModal.tsx
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.
概要
長押しで開く設定モーダルから、テーマ（自動・路線各社風・LED 等）を直接選べるテーマ一覧モーダルを追加した。これまでテーマ変更には設定画面まで遷移する必要があったが、走行中の画面から離れずに切り替えできるようにする。
変更の種類
変更内容
SelectBoundSettingListModalにthemeLabel/themeColor/onThemePressプロップを追加し、列車種別ボタンと同じ視覚で「テーマ」行（現在のテーマ名 + 色サンプル）を表示する。ThemeListModalを新規追加。getSettingsThemes()を用いて全テーマを色グラデーション + 「使用中 / 選ぶ」状態パネル付きで列挙し、LED テーマ時もデザインが破綻しないよう分岐済み。PermittedでthemePreferenceAtomから現在のテーマ設定を取得し、列車種別モーダルと同じ「設定モーダルを閉じてアニメーション完了後に次モーダルを開く」フローでテーマ一覧モーダルを開く。選択時はAsyncStorageへの保存 + atom 更新を行い、既存のThemeSettings画面と同じ永続化経路に揃える。SelectBoundSettingListModal.test.tsxにテーマ行の表示/非表示・押下時のコールバック呼び出し・themeLabel未指定時のフォールバック表示など 4 ケースを追加。テスト
npm run lintが通ることnpm testが通ることnpm run typecheckが通ること関連Issue
スクリーンショット（任意）
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes / UX
Tests