octopusthink / nautilus

Inclusive, open-source design system and React component library.
https://nautilus.octopusthink.com
MIT License
2 stars 0 forks source link

feat: Add noMargin prop to all components. #208

Closed sarahmonster closed 4 years ago

sarahmonster commented 4 years ago

A lot of our components have external margins—the thinking being that these provide a sensible default. You can just pop a component in and it'll be nicely spaced. It also helps with situations where Markdown is being converted to components, since we can't easily add CSS inside Markdown.

BUT the result is that I wind up spending a lot of time using CSS to zero out margins on components in layout files. This works fine in most cases, but we could simplify it still.

So now, instead of:

<Button
  css={css`
    margin: 0;
`} />

I can just write:

<Button noMargin />

...and the component will zero out any external margins.

I've also updated a lot of the READMEs to use this prop, because it makes things a lot cleaner. Yay!

Before

Screenshot 2020-03-22 at 20 33 51 Screenshot 2020-03-22 at 20 34 05

After

Screenshot 2020-03-22 at 20 32 42 Screenshot 2020-03-22 at 20 34 19

Fixes #163.

sarahmonster commented 4 years ago

Okay, I've now added tests, and they all pass! 🎊 I've looked through the snapshots and they all seem like they're doing what they should be, more or list.

List and Tabs are behaving a bit strangely—they wind up at the right place (margin 0) but take a sort of circituous route to get there, outputting margins three times:

exports[`List shouldn't have a margin when \`noMargin\` is true 1`] = `
.emotion-0 {
  margin: 0;
  margin: 0 0 0.4rem;
}
.emotion-2 {
  margin: 0;
}

I'm guessing this is something related to how these styles are being output via the bodyStyles const. I'm not sure if it's a problem here—it seems to work as expected on the frontend.