freenowtech / wave

Design System of FREE NOW
https://wave.free-now.com
Apache License 2.0
64 stars 22 forks source link

fix(tabbar): make Wave's theme apply to TabBar component #441

Closed martimalek closed 5 months ago

martimalek commented 5 months ago

What

Make Wave's theme apply to TabBar component

Media

{/* When passing a margin to TabBar it doesn't apply it correctly */}
{/* mb={3} should equal 24px (Spaces[3]), it is 16px instead */}
<TabBar mb={3}>
  {['Tab One', 'Tab Two'].map(it => (
    <TabBar.Link
        key={it}
        href="#"
        onClick={e => {
            e.preventDefault();
            setSelected(it);
        }}
        selected={selected === it}
    >
        {it}
    </TabBar.Link>
  ))}
</TabBar>
{/* Here's a correct margin for comparison */}
<Box width='100%' mb={3} >The margin above is 16px, but should be 24px</Box>
<Box width='100%' >The margin above is 24px</Box>
image

Check it out in this CodeSandbox

Why

All Wave components should have Wave's theme applied to them

How

github-actions[bot] commented 5 months ago

:tada: This PR is included in version 2.1.3 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

JanHamara commented 5 months ago

Hi @martimalek , I know this is already merged, just a little comment from what I noticed recently, and I see in your comment above TabBar that you noticed it too, maybe we should promote more to use the Spaces[value] that we have exported from Wave, rather than just numbers alone for margins, paddings, etc., because:

So I think it would make sense to also include using Spaces, when defining spacings of in our other components within the library

wdyt?