canvas-kit icon indicating copy to clipboard operation
canvas-kit copied to clipboard

Move spreading of CSS styles to the top of a style block

Open mannycarrera4 opened this issue 5 years ago • 1 comments

🐛 Bug Report

A clear and concise description of what the bug is. In many of our components we're spreading CSS style randomly in a style block, for example:

const ActionButton = styled('button')({
  display: 'block',
  backgroundColor: 'transparent', // To prevent Safari from rendering grey 'buttonface' as bgcolor
  border: 'none',
  padding: 0,
  marginTop: spacing.xxxs,
  ...type.body2,
  ...type.variant.link,
});

This creates issues because some of the the styles above the the spreading are overridden down below. In this case, the property display:block was overridden by type.variant.link which has a display: inline-block affecting some styling in the Toast component.

We should move the spreading of css styles to the top of the style block and then override any styles if necessary below it.

Proposed Change

const ActionButton = styled('button')({
  ...type.body2,
  ...type.variant.link,
  display: 'block',
  backgroundColor: 'transparent', // To prevent Safari from rendering grey 'buttonface' as bgcolor
  border: 'none',
  padding: 0,
  marginTop: spacing.xxxs,
});

This approach should also be part of our API guidelines and patterns

More info

Both in CK and storybook examples - My

Let's look into whether we can lint it easily (existing rule), to always spread first, and whether this is valid across our codebase. IF an existing lint rule does NOT exist this is out of scope for this ticket & should result in a separate ticket

mannycarrera4 avatar May 26 '20 16:05 mannycarrera4

Both in CK and storybook examples

myvuuu avatar Oct 07 '21 20:10 myvuuu