Fix component logic and layout issues#39
Conversation
- AnimatedList: add default keyExtractor fallback to prevent recycling issues - SegmentedControl: fix layout flash by waiting for containerWidth - Marquee: replace fragile setTimeout/measure with deterministic onLayout hooks Co-authored-by: truongnat <87919564+truongnat@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/ui/src/components/AnimatedList/AnimatedList.tsx">
<violation number="1" location="packages/ui/src/components/AnimatedList/AnimatedList.tsx:108">
P2: Use nullish coalescing for key fallback so valid falsy IDs (e.g. 0) are not replaced by index-based keys.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| ref={ref} | ||
| data={data} | ||
| renderItem={(info: any) => <AnimatedCell {...info} />} | ||
| keyExtractor={(item: any, index: number) => item?.id || item?.key || String(index)} |
There was a problem hiding this comment.
P2: Use nullish coalescing for key fallback so valid falsy IDs (e.g. 0) are not replaced by index-based keys.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ui/src/components/AnimatedList/AnimatedList.tsx, line 108:
<comment>Use nullish coalescing for key fallback so valid falsy IDs (e.g. 0) are not replaced by index-based keys.</comment>
<file context>
@@ -105,6 +105,7 @@ function AnimatedListInner<T>(
ref={ref}
data={data}
renderItem={(info: any) => <AnimatedCell {...info} />}
+ keyExtractor={(item: any, index: number) => item?.id || item?.key || String(index)}
{...flashListProps}
contentContainerStyle={useMemo(() => [
</file context>
| keyExtractor={(item: any, index: number) => item?.id || item?.key || String(index)} | |
| keyExtractor={(item: any, index: number) => String(item?.id ?? item?.key ?? index)} |
1 participant
Resolves multiple logical errors and layout issues across RNUI components including Marquee, AnimatedList, and SegmentedControl, aligning with stability improvements from the codebase audit.
PR created automatically by Jules for task 8189875729362001993 started by @truongnat
🔧 This PR fixes critical component logic and layout issues across three RNUI components (AnimatedList, Marquee, and SegmentedControl) by replacing fragile timing-based approaches with deterministic layout-driven solutions. The changes eliminate race conditions, improve rendering stability, and add proper fallback mechanisms for component recycling.
🔍 Detailed Analysis
Key Changes
keyExtractorfallback to prevent item recycling issues when data lacks proper identifierssetTimeout/measurepattern with deterministiconLayouthooks for container and content sizingsegmentWidthandcontainerWidthare available before showing indicator@shopify/flash-listv2.3.1 and updatedtsupto v8.5.1 across workspacesTechnical Implementation
sequenceDiagram participant App as Application participant AL as AnimatedList participant M as Marquee participant SC as SegmentedControl App->>AL: Render with data AL->>AL: Apply keyExtractor fallback Note over AL: item?.id || item?.key || String(index) App->>M: Mount Marquee M->>M: onLayout (container) M->>M: onLayout (content) M->>M: Start animation when both sizes available Note over M: Eliminates setTimeout race condition App->>SC: Render SegmentedControl SC->>SC: Wait for segmentWidth AND containerWidth SC->>SC: Show indicator only when both ready Note over SC: Prevents layout flashImpact
Created with Palmier
Summary by cubic
Fixes logic and layout issues in
Marquee,AnimatedList, andSegmentedControlto remove flicker, recycling glitches, and unreliable animations. Improves stability by using deterministic layout measurements and safer defaults.Bug Fixes
Marquee: Replace timeout/measure withonLayout; cache container/content sizes; start/resume only when sizes are known; cancel animations on unmount.AnimatedList: Add defaultkeyExtractor(item.id || item.key || String(index)) to avoid recycling issues with@shopify/flash-list.SegmentedControl: Show indicator only when bothsegmentWidthandcontainerWidthare ready to prevent layout flash.Dependencies
@shopify/flash-listto the workspace.Written for commit 043d54f. Summary will update on new commits.