-
Notifications
You must be signed in to change notification settings - Fork 55
feat(Flex): replace Flex.Gap with adding margins on the elements #1074
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1074 +/- ##
==========================================
- Coverage 81.78% 81.77% -0.02%
==========================================
Files 702 701 -1
Lines 8580 8564 -16
Branches 1245 1170 -75
==========================================
- Hits 7017 7003 -14
+ Misses 1548 1546 -2
Partials 15 15
Continue to review full report at Codecov.
|
} | ||
}, | ||
...(p.gap && { | ||
'> *:not(:last-child)': { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried several options for adding these styles.
- adding
marginLeft
andmarginTop
on all elements expect the first one (this however was conflicting with the push prop on theFlex.Item
, as it should override the same styles, but is applied first..) - I tried applying the margins which are not used (the
marginBottom
andmarginRight
), but with this I had to find figure out which element in the Flex is the last one which can be kind a tricky (we can have null values, or even the children in theFlex.Item
can be values - it seemed kind a risky...) - adding
gap
property on theFlex.Item
, which will just be propagated, and the inside theflexItemStyles
I was adding these margins before the styles of the push prop were provided). For the other child components inside theFlex
, I was providing the same styles. This introduced lots of duplication around the gap styles, and I didn't want to add new prop to the Flex.Item just because of the internal implementation.
That's how, I ended up with this, which seems to be the most simple solution, but I had to use this specific selector, which means it will always win over some inline styles, which may be surprising for the users... Let me know if you have some other ideas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let me, please, just point out couple of problems that we would need still address - but should mention explicitly that these were the problems of the original approach with Flex.Gap
as well. Both these problems appear when wrap
prop is truthy for Flex
. Lets suppose that flex direction is row
, wrapping is happening. Then
For some reason one of the examples for the Flex is difference, very minimal difference between the Flex.Items when the size prop is used.. (Anybody knows why is this happening?) |
Co-Authored-By: mnajdova <mnajdova@gmail.com>
Fixes the issue with rendering double gap between elements if the one between them is hidden. Does not render additional element for the gap => reduced amount of rendered dom elements.
Example code:
Previously:
Now: