fix: tighten header spacing, center dial, mute manual IP caret#32
Conversation
- Remove bottom padding from header, tighten side selector top padding - Vertically center dial + controls in remaining scroll area with Spacers - Reduce dial-to-controls spacing from 32 to 28 - Set DisclosureGroup chevron to Theme.textMuted via tint + accentColor so the caret matches the "Enter IP manually" label color
📝 WalkthroughWalkthroughThe PR updates UI styling and layout across two files. SleepypodApp.swift modifies the disconnected tab view's disclosure group appearance by restructuring the label with an HStack and adjusting accent colors. TempScreen.swift reorganizes the layout using spacers to vertically center the dial controls within the scrollable area. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
Sleepypod/SleepypodApp.swift (1)
485-486: Remove redundant.accentColor()modifier.Lines 485-486 apply both
.tint(Theme.textMuted)and.accentColor(Theme.textMuted)to the same control. With iOS 26.0 as the deployment target,.tint()alone is sufficient. The.accentColor()modifier is deprecated overlap and should be removed.♻️ Simplification
.tint(Theme.textMuted) - .accentColor(Theme.textMuted)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/SleepypodApp.swift` around lines 485 - 486, In SleepypodApp.swift, remove the redundant deprecated modifier by deleting the .accentColor(Theme.textMuted) call applied alongside .tint(Theme.textMuted) so the control only uses .tint(Theme.textMuted); keep the .tint modifier as the single source of truth for theming and ensure no other uses of .accentColor are left paired with .tint in the same view.Sleepypod/Views/Temp/TempScreen.swift (2)
61-74: Consider restoring a small gap between the selector and alerts.
Sleepypod/Views/Temp/SideSelectorView.swift:9-32only gives the selector card internal padding, so withVStack(spacing: 0)here an activeAlarmBannerwill now sit directly against it. A small bottom inset onSideSelectorView()or a conditional top inset on the alerts block would keep the two cards from visually merging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Views/Temp/TempScreen.swift` around lines 61 - 74, The selector and alert cards are touching because there’s no vertical gap between SideSelectorView() and the alerts VStack; update the layout to add a small gap when an AlarmBanner can appear by either adding a bottom inset to SideSelectorView (e.g., padding(.bottom, 8)) or applying a conditional top padding to the alerts VStack (e.g., if deviceManager.isAlarmActive then padding(.top, 8)) so AlarmBanner doesn’t visually merge with the selector; adjust the change around SideSelectorView, VStack(spacing: 8), and the AlarmBanner usage.
75-98: Don’t tie the centering math to a hard-coded60pt subtraction.Line 93 sizes the spacer layout from
geo.size.height - 60, but the header above thisScrollViewis not fixed-height (sideName,PrimingIndicator(),UserSelectorView(), Dynamic Type). That desynchronizes the centering from the real viewport, and it’s also worth re-checking with.refreshable+.scrollBounceBehavior(.basedOnSize). Please verify this on small/tall devices and with larger text, or deriveminHeightfrom the actual scroll viewport instead of a constant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Views/Temp/TempScreen.swift` around lines 75 - 98, The layout currently forces centering using a hard-coded subtraction (geo.size.height - 60) inside TempScreen which breaks when the header (sideName, PrimingIndicator, UserSelectorView, dynamic type) changes size; replace this magic constant by computing the actual available viewport height and subtracting the measured header height instead. Measure the header height (e.g., with a ViewPreferenceKey or a background GeometryReader) and use that headerHeight to set the ScrollView/VStack minHeight (instead of 60) so TemperatureDialView/TempControlsView/EnvironmentInfoView remain vertically centered across Dynamic Type, small/tall devices, and with .refreshable/.scrollBounceBehavior(.basedOnSize). Ensure the code references TempScreen, the GeometryReader that provides geo, and the header view(s) used to derive headerHeight when implementing the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sleepypod/SleepypodApp.swift`:
- Around line 485-486: In SleepypodApp.swift, remove the redundant deprecated
modifier by deleting the .accentColor(Theme.textMuted) call applied alongside
.tint(Theme.textMuted) so the control only uses .tint(Theme.textMuted); keep the
.tint modifier as the single source of truth for theming and ensure no other
uses of .accentColor are left paired with .tint in the same view.
In `@Sleepypod/Views/Temp/TempScreen.swift`:
- Around line 61-74: The selector and alert cards are touching because there’s
no vertical gap between SideSelectorView() and the alerts VStack; update the
layout to add a small gap when an AlarmBanner can appear by either adding a
bottom inset to SideSelectorView (e.g., padding(.bottom, 8)) or applying a
conditional top padding to the alerts VStack (e.g., if
deviceManager.isAlarmActive then padding(.top, 8)) so AlarmBanner doesn’t
visually merge with the selector; adjust the change around SideSelectorView,
VStack(spacing: 8), and the AlarmBanner usage.
- Around line 75-98: The layout currently forces centering using a hard-coded
subtraction (geo.size.height - 60) inside TempScreen which breaks when the
header (sideName, PrimingIndicator, UserSelectorView, dynamic type) changes
size; replace this magic constant by computing the actual available viewport
height and subtracting the measured header height instead. Measure the header
height (e.g., with a ViewPreferenceKey or a background GeometryReader) and use
that headerHeight to set the ScrollView/VStack minHeight (instead of 60) so
TemperatureDialView/TempControlsView/EnvironmentInfoView remain vertically
centered across Dynamic Type, small/tall devices, and with
.refreshable/.scrollBounceBehavior(.basedOnSize). Ensure the code references
TempScreen, the GeometryReader that provides geo, and the header view(s) used to
derive headerHeight when implementing the fix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c0b105d6-62a4-414c-b770-f5722546a810
📒 Files selected for processing (2)
Sleepypod/SleepypodApp.swiftSleepypod/Views/Temp/TempScreen.swift
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit