fix(just-lift): avoid weight(1f) on weight cards inside verticalScroll (#574 follow-up) - #576
Closed
cursor[bot] wants to merge 2 commits into
Closed
fix(just-lift): avoid weight(1f) on weight cards inside verticalScroll (#574 follow-up)#576cursor[bot] wants to merge 2 commits into
cursor[bot] wants to merge 2 commits into
Conversation
…n iPhone portrait (#571) Issue #571 reporter (iPhone 16 Pro, Just Lift, LB) saw the Weight Change Per Rep slider produce 29/30/44-49 instead of 1 lb increments. RCA proved the slider is correctly bound to valueRange = -10..+10 with 1 lb steps and cannot emit those values; the visible jumps were the iOS CompactNumberPicker (LazyColumn wheel, range 1..242 LB) claiming vertical drags intended for the slider and snapping to neighbour weights. This commit applies the three-part fix from the GPT-5.5 RCA: 1. WindowSizeClass.kt + JustLiftScreen.kt — split the side-by-side weight card gate from the broader useCompactAccessibility. New shouldStackWeightCards() returns true only when the screen is also short (height Compact), so iPhone portrait (402x874dp) with default Dynamic Type / Bold Text keeps the two weight cards side-by-side. iPhone landscape and short-height tablets still stack. 2. JustLiftScreen.kt — add an explicit Spacer(Spacing.small) between the two weight cards when stacked, belt-and-braces separation. 3. CompactNumberPicker.ios.kt — add a pointerInput on the wheel's BoxWithConstraints that consumes vertical drags landing outside the centred 36dp selected band, so the inner LazyColumn does not claim them. In edit mode the wheel defers entirely to the BasicTextField. Adds WindowSizeClassTest cases for shouldStackWeightCards and a new JustLiftScreenWeightSliderWiringTest that pins the ProgressionSlider's valueRange and weightChangePerRep binding against future regressions. Refs: #571 (Fixes) RCA: rca_owner=gpt-5.5-xhigh, fix_driver=gpt55_rca
PR #574 split stackWeightCards from useCompactAccessibility for issue #571, but Bold Text / large Dynamic Type still enables outer verticalScroll while stackWeightCards stays false on tall portrait. Applying Modifier.weight(1f) inside a scrolling Column uses unbounded height constraints and can crash at layout time. Gate intrinsic-height layout on useCompactAccessibility || stackWeightCards and add a source-level regression test. Co-authored-by: Devil <9thLevelSoftware@users.noreply.github.com>
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.
Bug and impact
PR #574 (#571 fix) introduced a layout crash on iPhone portrait with Bold Text or large Dynamic Type enabled.
When
useCompactAccessibilityis true, the outerColumnusesverticalScroll. The PR changed weight-card modifiers to gateModifier.weight(1f)only onstackWeightCards, butstackWeightCardsstays false on tall portrait (by design, to avoid the wheel/slider gesture conflict). That leavesweight(1f)on children inside a vertically scrolling parent — an invalid Compose constraint that can fail layout with unbounded-height measure errors.Trigger: iPhone portrait → Settings → Accessibility → Display & Text Size → Bold Text ON (or Dynamic Type ≥ ~1.15×) → Just Lift → Old School/Pump mode.
Root cause
Two independent layout gates were split (
useCompactAccessibilityfor scroll,stackWeightCardsfor card sizing) without accounting for the intersection case where scroll is on but stacking is off.Fix
Introduce
weightCardsUseIntrinsicHeight = useCompactAccessibility || stackWeightCardsand use it for weight-cardweight/fillMaxSizemodifiers.Modifier.weight(1f)is only applied when neither outer scroll nor short-height stacking applies (default portrait).Added a source-level regression test in
JustLiftScreenWeightSliderWiringTest.Validation
weight(1f)+verticalScrollinvariantWindowSizeClassTestcases unchanged