epam / UUI

React-based components and accelerators library built by EPAM Systems.
https://uui.epam.com/
MIT License
180 stars 67 forks source link

[Loveship][FlexRow]: remove additional margings #2581

Open JuliaMV opened 1 week ago

JuliaMV commented 1 week ago

Steps to Reproduce

  1. open https://uui.epam.com/documents?id=flexRow&mode=propsEditor&isSkin=true&category=flexItems
  2. select theme: loveship
  3. select props: children: 'Text, TextInput, Button' and columnGap: 6

Actual result

actual space between elements is 12px 2024-10-24_10h56_08

Expected result

space between elements is 6px

vasilii-kovalev commented 1 week ago

I can see that default spacing values are defined here. It can be removed by passing null value on the user's end. spacing property has been deprecated:

Does it make sense to remove the default values, if the property itself will be removed anyway? Plus, it would be a breaking change. P.S.: I'm not a part of the team, just a little familiar with the topic 😊

AlekseyManetov commented 1 week ago

Does it make sense to remove the default values, if the property itself will be removed anyway? Plus, it would be https://github.com/epam/UUI/issues/2034#issuecomment-1994575382.

I believe we should do it together with prop removing, because default value change would already be a breaking change itself.

The current situation is also problematic because user need to provide spacing={ null } along with columnGap to disable the warning and spacing default behavior. On the other hand, we want this change to be reviewed and managed by UUI clients, as it may require additional adjustments on the project side in some cases.

AlekseyManetov commented 5 days ago

Finally we've decided to replace default value from spacing to columnGap for loveship skin, with a small breaking change for corner case. Will release it in a next release - https://github.com/epam/UUI/commit/77c7ea210595e7bd5dae23fe683af0e8030e42a3.