fix: onboarding button not rendering on macOS 15 (#420)#426
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughFixed onboarding UI rendering on macOS 15 by adding explicit frame/alignment constraints and simplifying transitions; updated changelog to document the fix. Adjusted bottom navigation animation duration and transition behavior in the onboarding view. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
TablePro/Views/Connection/OnboardingContentView.swift (1)
186-188: Make hidden Skip button non-interactive on the last page.At Line 186,
opacity(0)keeps the button hittable/focusable; with the newminWidthat Line 187, the invisible hit target is larger and can still trigger onboarding completion.♻️ Suggested tweak
.opacity(currentPage == 2 ? 0 : 1) .frame(minWidth: 110, alignment: .leading) + .allowsHitTesting(currentPage < 2) + .accessibilityHidden(currentPage == 2)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TablePro/Views/Connection/OnboardingContentView.swift` around lines 186 - 188, The Skip button remains hittable when hidden because only opacity is changed; update the Skip button in OnboardingContentView to also disable hit testing when currentPage == 2 by adding .allowsHitTesting(currentPage != 2) (or .disabled(currentPage == 2)) alongside the existing .opacity(currentPage == 2 ? 0 : 1) and .frame(minWidth: 110, alignment: .leading) so the invisible button cannot be interacted with on the last page.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@TablePro/Views/Connection/OnboardingContentView.swift`:
- Around line 186-188: The Skip button remains hittable when hidden because only
opacity is changed; update the Skip button in OnboardingContentView to also
disable hit testing when currentPage == 2 by adding
.allowsHitTesting(currentPage != 2) (or .disabled(currentPage == 2)) alongside
the existing .opacity(currentPage == 2 ? 0 : 1) and .frame(minWidth: 110,
alignment: .leading) so the invisible button cannot be interacted with on the
last page.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10288a47-5104-4d7f-b36e-dec6cbb15104
📒 Files selected for processing (2)
CHANGELOG.mdTablePro/Views/Connection/OnboardingContentView.swift
Summary
GroupwithZStackfor the Continue/Get Started button container —Groupis not a layout boundary and cannot drive structuralif/elsetransitions on macOS 15.transition(.opacity)to both button branches (was only on "Get Started") for symmetric animationframe(minWidth: 110)to both edge slots (Skip button and action button) to center dot indicatorsCloses #420
Test plan
Summary by CodeRabbit