Skip to content

Conversation

@peachbits
Copy link
Contributor

@peachbits peachbits commented Dec 4, 2025

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

Dependencies

none

Requirements

If you have made any visual changes to the GUI. Make sure you have:

  • Tested on iOS device
  • Tested on Android device
  • Tested on small-screen device (iPod Touch)
  • Tested on large-screen device (tablet)

Note

Prevents a blank white square by showing a spinner until layout/data are ready and animating QR opacity via a shared value.

  • UI (QR Code src/components/themed/QrCode.tsx):
    • Loading state: Add layoutPending to gate rendering; show ActivityIndicator until layout size is known and data is present.
    • Animation: Replace useDerivedValue-driven timing with useSharedValue(opacity) updated in useEffect; useAnimatedStyle now reads opacity directly.
    • Conditional render: Render Animated.View (QR + center icon) only when ready, preventing an empty white square flicker.
    • QR generation and icon sizing/layout unchanged.

Written by Cursor Bugbot for commit b370540. This will update automatically on new commits. Configure here.


@Jon-edge
Copy link
Collaborator

Jon-edge commented Dec 5, 2025

Noted that there's still a re-layout issue due to the varying sizes of the receive address text on the bottom of the scene pushing the QR, but outside of the scope of this fix.

<ActivityIndicator color={theme.iconTappable} />
<Animated.View style={[styles.whiteBox, fadeStyle]}>
{size <= 0 ? null : (
{size <= 0 ? (
Copy link
Collaborator

@Jon-edge Jon-edge Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems kind of hacky using size for loading logic here but I get it. I would prefer an explicit var to make it more obvious (i.e. const layoutPending = size <= 0 // loading state includes layout timing to avoid flicker)

Should also guard against || data == null for completeness, regardless if data == null is even possible - this future-proofs it.


React.useEffect(() => {
opacity.value = withTiming(data != null ? 1 : 0)
}, [data, opacity])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Fade animation completes before QR code view renders

The opacity animation triggers on data changes, but the Animated.View only renders when layoutPending is false (requiring both size > 0 and data != null). If data is set before layout completes, the animation runs while the ActivityIndicator is still showing, completing before the QR code view mounts. This causes the QR code to pop in without the intended fade effect, since the animation dependency array doesn't include size or layoutPending.

Fix in Cursor Fix in Web


React.useEffect(() => {
opacity.value = withTiming(data != null ? 1 : 0)
}, [data, opacity])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Fade animation completes before QR code view renders

The opacity animation triggers on data changes, but the Animated.View only renders when layoutPending is false (requiring both size > 0 and data != null). If data is set before layout completes, the animation runs while the ActivityIndicator is still showing, completing before the QR code view mounts. This causes the QR code to pop in without the intended fade effect, since the animation dependency array doesn't include size or layoutPending.

Fix in Cursor Fix in Web

@Jon-edge
Copy link
Collaborator

Fade seems fine under current conditions, can fix the bugbot comments or not.

@peachbits peachbits enabled auto-merge December 11, 2025 21:08
@peachbits peachbits merged commit 6abcc0d into develop Dec 11, 2025
3 checks passed
@peachbits peachbits deleted the matthew/qr-code branch December 11, 2025 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants