Repository navigation
[feature/ui] Preliminary UI design - #1
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe app adds a themed alarm-management interface with typed alarm data, creation and editing forms, interactive task controls, countdown utilities, updated task screens, dark Expo branding, and staged linting configuration. ChangesAlarm foundation
Alarm creation and management
Task screens and application setup
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Home
participant AlarmCard
participant EditAlarmScreen
participant CreateAlarmForm
participant useCreateAlarmForm
Home->>AlarmCard: render alarm actions
AlarmCard->>Home: report selection or navigation press
Home->>EditAlarmScreen: navigate with alarm id
EditAlarmScreen->>CreateAlarmForm: render edit form
CreateAlarmForm->>useCreateAlarmForm: update and confirm form
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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: 5
🧹 Nitpick comments (6)
src/components/CreateAlarmForm.tsx (1)
85-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
getDaysStringfor richer feedback.
helperTextshows the generic "Selected days" instead of listing which days are selected.getDaysString(already used inAlarmCard) would show the actual selection.♻️ Suggested fix
- <Text style={styles.helperText}> - {form.weekdaysSelected.length === 0 ? "Every day" : "Selected days"} - </Text> + <Text style={styles.helperText}> + {getDaysString(form.weekdaysSelected)} + </Text>🤖 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/CreateAlarmForm.tsx` around lines 85 - 87, Update the helperText rendering in CreateAlarmForm to reuse the existing getDaysString helper, displaying the selected weekday names while preserving the “Every day” output when weekdaysSelected is empty.src/components/AlarmCard.tsx (1)
32-36: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCard is missing accessibility metadata.
The outer
Pressable(primary interactive element for select/navigate) has noaccessibilityRoleoraccessibilityLabeldescribing the alarm time/days/state, unlikeWeekdayTextButtonelsewhere in this PR.♿ Suggested fix
<Pressable style={({ pressed }) => [styles.card, pressed && styles.cardPressed]} onPress={() => onPress(alarm)} onLongPress={() => onLongPress(alarm)} + accessibilityRole="button" + accessibilityLabel={`Alarm at ${formatTime(alarm.hour, alarm.minute)}, ${getDaysString(alarm.days)}`} >🤖 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/AlarmCard.tsx` around lines 32 - 36, The outer Pressable in AlarmCard needs accessibility metadata. Add an appropriate accessibilityRole and a descriptive accessibilityLabel that communicates the alarm time, scheduled days, and enabled/disabled state, while preserving the existing press handlers.src/components/TimeWheelPicker.tsx (1)
37-89: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWheel items lack accessibility labels.
Unlike
WeekdayTextButtonin this same PR, the per-rowPressableitems here have noaccessibilityRole/accessibilityLabel, so screen-reader users can't identify or select hour/minute/period values.♿ Suggested fix
<Pressable key={item} style={styles.item} onPress={() => handleItemPress(i)} + accessibilityRole="button" + accessibilityState={{ selected: i === index }} + accessibilityLabel={item} >🤖 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/TimeWheelPicker.tsx` around lines 37 - 89, Update the per-item Pressable in Wheel to include an appropriate accessibilityRole and an accessibilityLabel derived from the rendered item value, so screen readers identify each hour, minute, or period option and allow users to select it.src/hooks/useCreateAlarmForm.ts (2)
214-238: 🎯 Functional Correctness | 🔵 TrivialConfirm currently discards the draft alarm.
buildAlarmis referenced only viavoid buildAlarm;and never called beforerouter.back(), so creating/editing an alarm doesn't persist anything yet. This matches the documentedpersistAlarmplaceholder, so likely intentional for this UI-only pass — flagging to make sure it's tracked before this ships.🤖 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/hooks/useCreateAlarmForm.ts` around lines 214 - 238, Implement the persistAlarm flow in handleConfirm instead of discarding the draft: call buildAlarm, persist the resulting alarm through the appropriate create or update path, reschedule it, and navigate back only after persistence completes. Replace the void buildAlarm placeholder while preserving the existing no-reset navigation behavior.
78-93: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
alarmSoundSelectedwill show a raw file path instead of a display name.
fromAlarmsetsalarmSoundSelectedtoalarm.soundwhen custom, butAlarm.soundonly stores the file URI (perpickSound's own plan,alarmSoundUrigets thefile://path andalarmSoundSelectedshould getasset.name). Once sound picking is implemented, re-opening an alarm with a custom sound will render the raw URI as the label. Consider storing a separate display name field onAlarm(e.g.soundName) so round-tripping through edit doesn't lose the friendly name.🤖 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/hooks/useCreateAlarmForm.ts` around lines 78 - 93, The custom sound label in fromAlarm must not use Alarm.sound, since that field is the file URI. Add and persist a separate display-name field on Alarm (such as soundName), set it when pickSound selects an asset, and use it for alarmSoundSelected while retaining alarm.sound for alarmSoundUri; preserve “Default” behavior for built-in sounds.src/app/(main)/index.tsx (1)
111-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated select-all condition.
selectedIds.size === alarms.length && alarms.length > 0is computed separately for theaccessibilityLabeland theText. Extracting a localallSelectedvariable avoids the duplication.♻️ Suggested fix
+ {(() => { + const allSelected = selectedIds.size === alarms.length && alarms.length > 0; + return ( <Pressable accessibilityRole="button" - accessibilityLabel={ - selectedIds.size === alarms.length && alarms.length > 0 - ? "Unselect all" - : "Select all" - } + accessibilityLabel={allSelected ? "Unselect all" : "Select all"} onPress={selectAll} > - <Text style={styles.actionRowText}> - {selectedIds.size === alarms.length && alarms.length > 0 - ? "Unselect all" - : "Select all"} - </Text> + <Text style={styles.actionRowText}> + {allSelected ? "Unselect all" : "Select all"} + </Text> </Pressable> + ); + })()}🤖 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/app/`(main)/index.tsx around lines 111 - 125, In the component containing the select-all Pressable, extract the repeated `selectedIds.size === alarms.length && alarms.length > 0` expression into a local `allSelected` variable, then reuse it for both `accessibilityLabel` and the action-row `Text` while preserving the existing labels and behavior.
🤖 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 @.husky/pre-commit:
- Line 1: Update the pre-commit hook’s lint-staged invocation to use the locally
installed binary, such as ./node_modules/.bin/lint-staged or npx --no
lint-staged, preventing fallback to the package registry.
In `@src/app/`(main)/create-alarm/[alarmId].tsx:
- Around line 21-25: Update the render condition around the alarm loading view
to distinguish active loading from a completed lookup with no alarm. Keep the
loading indicator for loading states, and when loading is false and alarm is
null, render a not-found recovery state or navigate back to the appropriate
create flow.
In `@src/app/`(main)/index.tsx:
- Around line 98-174: Add flex: 1 to the alarm list’s wrapping View and, if
necessary, the FlatList in the screen component so the list participates in the
available-height flex chain and scrolls independently for long alarm
collections. Keep the existing list data, rendering, and styles unchanged.
In `@src/components/FormSlider.tsx`:
- Around line 35-50: Update the PanResponder callbacks in FormSlider’s
applyFraction flow to use event.nativeEvent.locationX rather than
gestureState.x0/moveX, and normalize the coordinate against usable instead of
trackWidth. Preserve the existing clamping, snapping, and disabled behavior
while ensuring values are calculated from the slider’s local coordinate system.
In `@src/components/RadioButton.tsx`:
- Around line 29-36: Update the selected-state rendering in RadioButton so the
selection dot remains visible when disabled; render the dot whenever selected is
true, while preserving the existing circleDisabled styling for disabled options.
---
Nitpick comments:
In `@src/app/`(main)/index.tsx:
- Around line 111-125: In the component containing the select-all Pressable,
extract the repeated `selectedIds.size === alarms.length && alarms.length > 0`
expression into a local `allSelected` variable, then reuse it for both
`accessibilityLabel` and the action-row `Text` while preserving the existing
labels and behavior.
In `@src/components/AlarmCard.tsx`:
- Around line 32-36: The outer Pressable in AlarmCard needs accessibility
metadata. Add an appropriate accessibilityRole and a descriptive
accessibilityLabel that communicates the alarm time, scheduled days, and
enabled/disabled state, while preserving the existing press handlers.
In `@src/components/CreateAlarmForm.tsx`:
- Around line 85-87: Update the helperText rendering in CreateAlarmForm to reuse
the existing getDaysString helper, displaying the selected weekday names while
preserving the “Every day” output when weekdaysSelected is empty.
In `@src/components/TimeWheelPicker.tsx`:
- Around line 37-89: Update the per-item Pressable in Wheel to include an
appropriate accessibilityRole and an accessibilityLabel derived from the
rendered item value, so screen readers identify each hour, minute, or period
option and allow users to select it.
In `@src/hooks/useCreateAlarmForm.ts`:
- Around line 214-238: Implement the persistAlarm flow in handleConfirm instead
of discarding the draft: call buildAlarm, persist the resulting alarm through
the appropriate create or update path, reschedule it, and navigate back only
after persistence completes. Replace the void buildAlarm placeholder while
preserving the existing no-reset navigation behavior.
- Around line 78-93: The custom sound label in fromAlarm must not use
Alarm.sound, since that field is the file URI. Add and persist a separate
display-name field on Alarm (such as soundName), set it when pickSound selects
an asset, and use it for alarmSoundSelected while retaining alarm.sound for
alarmSoundUri; preserve “Default” behavior for built-in sounds.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a7e08b55-071b-472a-a946-0a873a954680
⛔ Files ignored due to path filters (5)
assets/fonts/Geist-Bold.ttfis excluded by!**/*.ttfassets/fonts/Geist-Medium.ttfis excluded by!**/*.ttfassets/fonts/Geist-Regular.ttfis excluded by!**/*.ttfassets/fonts/Geist-SemiBold.ttfis excluded by!**/*.ttfpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (30)
.husky/pre-commitapp.jsondocs/07-react-native-migration-guide.mdpackage.jsonsrc/app/(alarm)/_layout.tsxsrc/app/(alarm)/tasks/math-equation/[rounds]/[difficulty].tsxsrc/app/(alarm)/tasks/memory-game/[rounds]/[difficulty].tsxsrc/app/(alarm)/tasks/phone-shaking.tsxsrc/app/(main)/_layout.tsxsrc/app/(main)/create-alarm/[alarmId].tsxsrc/app/(main)/create-alarm/index.tsxsrc/app/(main)/index.tsxsrc/app/_layout.tsxsrc/components/AlarmCard.tsxsrc/components/CreateAlarmForm.tsxsrc/components/FormSlider.tsxsrc/components/RadioButton.tsxsrc/components/TimeWheelPicker.tsxsrc/components/WeekdayTextButton.tsxsrc/data/constants.tssrc/data/mockAlarms.tssrc/data/types.tssrc/hooks/useAlarmById.tssrc/hooks/useCreateAlarmForm.tssrc/theme/colors.tssrc/theme/index.tssrc/theme/radii.tssrc/theme/spacing.tssrc/theme/typography.tssrc/utils/time.ts
npx can fall back to the registry when the binary is missing locally. Use `npx --no` so the hook fails closed instead of fetching from the registry.
A resolved-but-null alarm previously kept the loading indicator on screen indefinitely. Render a not-found recovery state with a link to the create flow when loading is false and no alarm was found.
The list wrapper had no flex sizing, so a long alarm collection expanded to content height and got clipped instead of scrolling. Add flex:1 to the wrapper so the FlatList participates in the available-height flex chain.
gestureState.x0/moveX are screen-relative, so the selected value shifted with page offset and could miss the minimum. Use the responder View's local locationX and normalize against the usable track width.
A selected disabled option rendered without its dot, leaving its current state unclear. Render the dot whenever selected, dimmed via dotDisabled for disabled options.
This was referenced Jul 30, 2026
This was referenced Aug 11, 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
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.
Summary by CodeRabbit