search

wix / react-native-ui-lib

UI Components Library for React Native
https://wix.github.io/react-native-ui-lib/
MIT License
6.59k stars 717 forks source link

StackAggregator - several fixes #3210

Open M-i-k-e-l opened 3 months ago

M-i-k-e-l commented 3 months ago

Description

Strictly speaking this is a breaking change (see change log notes), let me know what you think.

Although the previous code for the index comparison seemed correct, it did not work correctly on Android, the new logic works on both platforms (except on an edge case were the user is mixing controlled and uncontrolled usage).

For visibility I'll add the bug reproduction steps:

  1. Restart the StackAggregatorScreen.
  2. Press on the Open Stack button.
  3. Press on the Show less button.
  4. Press on the Close Stack button. Result - the edges of the cards "become straight" (this is actually the scale animation failing), I was not able to fix this and it seemed too small to invest more time into.

Changelog

🚫 StackAggregator - fix card being transparent on Android - BREAKING CHANGE - Cards' backgroundColor should now be passed via the backgroundColor prop and not via the contentContainerStyle prop StackAggregator - onCollapseChanged will now be called after the animation has ended (as was intended)

Additional info

Ticket 4186

Inbal-Tish commented 3 months ago

@M-i-k-e-l I was wondering if there's a simpler way, like moving the 'backgroundColor: 'transparent' to the Animated.View and the contentContainerStyle to the Card's style. What do you think?

M-i-k-e-l commented 2 months ago

@M-i-k-e-l I was wondering if there's a simpler way, like moving the 'backgroundColor: 'transparent' to the Animated.View and the contentContainerStyle to the Card's style. What do you think?

It causes another bug when adding a card.

Marking with v8

Inbal-Tish commented 2 months ago

@M-i-k-e-l I was wondering if there's a simpler way, like moving the 'backgroundColor: 'transparent' to the Animated.View and the contentContainerStyle to the Card's style. What do you think?

It causes another bug when adding a card.

Marking with v8

Did you investigate the bug created when adding a card? Is it a complex fix?

M-i-k-e-l commented 2 months ago

@M-i-k-e-l I was wondering if there's a simpler way, like moving the 'backgroundColor: 'transparent' to the Animated.View and the contentContainerStyle to the Card's style. What do you think?

It causes another bug when adding a card. Marking with v8

Did you investigate the bug created when adding a card? Is it a complex fix?

If you mean the bug with the shadows then I've committed it already (sadly it did not work well in master).

Inbal-Tish commented 2 months ago

@M-i-k-e-l I was wondering if there's a simpler way, like moving the 'backgroundColor: 'transparent' to the Animated.View and the contentContainerStyle to the Card's style. What do you think?

It causes another bug when adding a card. Marking with v8

Did you investigate the bug created when adding a card? Is it a complex fix?

If you mean the bug with the shadows then I've committed it already (sadly it did not work well in master).

Not what I meant. The shadow bug is on master. With my suggested fix there's a new bug where the cards disappear on iOS. I meant if you investigated this new one? Maybe it's a small fix?