Skip to content

Jon/fix/ios-carousel #5548

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Jon/fix/ios-carousel #5548

wants to merge 2 commits into from

Conversation

Jon-edge
Copy link
Collaborator

@Jon-edge Jon-edge commented Apr 25, 2025

In the previous change that attempted to fix flickering of the promo card, dataLocal as a dependency was causing an infinite loop for iOS repeatedly clearing the data every 500ms.

The rendering hacks was incorrectly verified as no longer being necessary. and removed.

The hacks are still necessary, but need only trigger once on mount.

Delay also reduced to be less perceptible.

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)

Copy link
Contributor

@swansontec swansontec left a comment

Choose a reason for hiding this comment

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

One cleanup suggestion.

Also, please squash the two commits into one. The changes in the first commit are broken without the second commit, so there's no point in keeping them apart.

Comment on lines 49 to 53
const tempData = [...dataLocal]
setDataLocal([])
setTimeout(() => {
setDataLocal(tempData)
}, 50)
Copy link
Contributor

Choose a reason for hiding this comment

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

What about:

setDataLocal([])
setTimeout(() => setDataLocal(data), 50)

This would let the hook's dependencies actually match the deps array, and would also get rid of the tempData variable.

Reinstate rendering hack.

`dataLocal` as a dependency was causing an infinite loop for iOS repeatedly clearing the data every 500ms.

The rendering hacks need only trigger once on mount.

Delay also reduced to be less perceptible.
@Jon-edge Jon-edge force-pushed the jon/fix/ios-carousel branch from ad640d7 to 1339099 Compare April 28, 2025 04:51
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.

2 participants