Skip to content

Conversation

landabaso
Copy link
Contributor

Description

This PR addresses an issue with the Slider component where a flicker effect is visible during re-renders, specifically in the web implementation. The flicker was caused by the width being reset during the layout calculation.

Changes Made

  • Modified sliderStyle to set the width only if it is not zero using the conditional spread operator.
    const sliderStyle = { zIndex: 1, ...(width !== 0 && { width }) };

- Updated `sliderStyle` to conditionally set width only when it's not zero to prevent initial flicker during re-renders.
- This change ensures a smoother user experience by eliminating flicker effects when the component re-renders.
@landabaso
Copy link
Contributor Author

Oups, one of the tests failed (due to a snapshot comparison). Please let me know if you agree with this PR approach, and I'll update the snapshot accordingly.

@landabaso
Copy link
Contributor Author

landabaso commented May 21, 2024

In fact, I am still experiencing flicker issues on iOS and Android. Not setting the width at all (as it was originally) solves the flicker. That is: const sliderStyle = { zIndex: 1 }; However, I assume the width was set for a specific reason, which is why I didn't remove it in my initial approach. Is setting the width strictly needed?

Could you provide some insights on why the width was added, @BartoszKlonowski? This was introduced in PR #555. Setting styles in onLayout can be tricky and may introduce flickering, depending on how parent components handle their children.

Thanks!

@simon-thuresson-md
Copy link

I would like to get more attention to this PR. I just recently turned on the the new arch had quite a lot of flickering issues from this slider library. Removing the width as @landabaso suggests solved the issues completely while, as far as I can tell, leave everything else still working. I am currently using a patch in order to fix my flickering issues. Removing the width or an explanation to why it is needed (or another fix entirely) would be awesome.

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