fix: collapse a checked checklist row as one piece#3595
Conversation
Checking an item off under the Open filter played a scrambled exit: the row shrank away while the checkbox held its full size and got sliced in half by the shrinking row, and the row's contents jumped sideways the instant the animation began. The hide wrapper was an AnimatedCrossFade to a zero-sized second child, which has two independent problems: - It collapses via AnimatedSize, which lays the child out at natural size and animates only the box, clipping the overflow. At 25% of the 300ms collapse the row's bottom sat at y=45.8 while the checkbox's bottom sat at y=54.0 — hanging below the visible row and cropped. At 50% the row was at half height and the checkbox still at 100%. - Flipping crossFadeState makes the outgoing row stop being the sizing child, so it is re-laid-out against the zero-sized second child's constraints. On the very first frame, before any size animation, the checkbox jumped from x=64 to x=247 and y=28 to y=46. Under the card's loose width constraints the box also narrowed from both sides (366 -> 319 -> 183 px). Replace it with SizeFadeCollapse, a new design-system motion primitive and the exit counterpart to SizeFadeEntrance. It drives reserved height, paint scale and opacity from one tween sharing one anchor (top-start), so painted size equals reserved size on every frame: nothing is cropped, the box keeps full width, and the whole row shrinks together. Reduced motion snaps. A collapsing row is IgnorePointer'd / ExcludeFocus'ed / ExcludeSemantics'ed immediately but keeps ticking until the collapse ends, so an in-flight strike-through wipe or checkbox pop is not frozen half-played. duration is required rather than defaulted because task_details_page computes its scroll-anchor delay from checklistCompletionAnimationDuration + checklistCompletionFadeDuration; a silent default would let the two drift. Post-fix geometry across the collapse (row height / checkbox / inside row): 0ms 52.0 20.0x20.0 true; 25ms 46.8 18.0x18.0 true; 50ms 30.8 11.9x11.9 true; 75ms 20.4 7.8x7.8 true. Adds four regression tests to checklist_item_row_test.dart, all of which fail against the old implementation, plus eight tests for the new component.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3595 +/- ##
==========================================
+ Coverage 94.37% 99.15% +4.78%
==========================================
Files 1783 1784 +1
Lines 130926 130978 +52
==========================================
+ Hits 123559 129872 +6313
+ Misses 7367 1106 -6261
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Checking an item off under the Open filter played a scrambled exit: the row shrank away while the checkbox held its full size and got sliced in half by the shrinking row, and the row's contents jumped sideways the instant the animation began.
Root cause
The hide/show wrapper in
checklist_item_row_state.dartwas anAnimatedCrossFadefrom the row to a zero-sizedSizedBox.shrink. That has two independent defects, both measured rather than inferred:1. It clips, it doesn't scale.
AnimatedCrossFadecollapses viaAnimatedSize, which lays the child out at its natural size and animates only the box, clipping the overflow. The fixed 44×44 checkbox therefore kept its full dimensions while the band closed over it.2. The outgoing row is re-laid-out against zero-size constraints. When
crossFadeStateflips, the row stops being the sizing child and is laid out against the zero-sized second child. On the very first frame, before any size animation has run, the checkbox jumped from x=64 to x=247 and y=28 to y=46. Under the card's loose width constraints the box also collapsed horizontally (366 → 319 → 183 px), pulling the row in from both sides towardAlignment.topCenter.Fix
New design-system motion primitive
SizeFadeCollapse(components/motion/), the exit counterpart toSizeFadeEntrance. It drives reserved height, paint scale and opacity from one tween sharing one anchor (top-start), so painted size equals reserved size on every frame. Nothing is cropped, the box keeps full width, and the row leaves as a single piece.Behaviour preserved from the old wrapper: reduced motion snaps instead of animating; a collapsing row is
IgnorePointered /ExcludeFocused /ExcludeSemanticsed as soon as it starts leaving, so it can neither be tapped nor announced. Its subtree keeps ticking until the collapse finishes, so an in-flight strike-through wipe or checkbox pop is not frozen half-played. The collapse is reversible — unchecking runs the same tween backwards.durationis required rather than defaulted:task_details_page.dartcomputes its scroll-anchor delay fromchecklistCompletionAnimationDuration + checklistCompletionFadeDuration, and a silent default would let the two drift apart unnoticed.Verification
Geometry across the collapse after the fix — the checkbox now shrinks with the row and stays inside it at every step, and the row's width never changes:
Confirmed visually too, by rendering the animation frame by frame through the in-app screenshot harness (throwaway capture test, not committed).
checklist_item_row_test.dart, each verified to fail against the old implementationSizeFadeCollapse(geometry invariant per frame, leading-edge anchor, reverse, reduced motion, pointer/semantics exclusion, duration retiming)AnimatedCrossFade.crossFadeStatemigrated toSizeFadeCollapse.collapsed2542 tests pass(1 skipped), analyzer clean, formatter cleanfeatures/tasks,features/design_system), CHANGELOG + flatpak metainfo updated under 0.9.1069Deliberately not done
SizeFadeEntrancehas the same clip-not-scale geometry, so a row entering the list is revealed by cropping rather than scaling — insert and remove are now asymmetric. That is outside the reported bug and changing it is a product call, so it is flagged rather than folded in. Related:checklist_card.dartalready carries a comment aboutAnimatedCrossFadesqueezing an outgoing body at progressively narrower widths — the same defect class, worked around there by keeping a width-stable hidden endpoint.Bead:
lotti3-tp0