Skip to content
This repository has been archived by the owner on Sep 21, 2023. It is now read-only.

refactor: Use variables for button padding, font and weight #625

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Fumler
Copy link
Contributor

@Fumler Fumler commented Jul 31, 2018

Please tick a box

  • refactor (A code change that neither fixes a bug nor adds a feature)

If you're adding a feature, is it documented in a storybook story?

  • Yes

Are the components you're working on reusable between brands?

  • Yes (Using variables that are defined in the default theme)

@Fumler Fumler requested a review from misund July 31, 2018 15:51
@misund
Copy link
Contributor

misund commented Jul 31, 2018

Interesting approach to nested variables!

@misund
Copy link
Contributor

misund commented Jul 31, 2018

I like the customization options here! Not so much a fan of that these values are not connected to the rest of the design pattern. I wish we could have both.

@misund
Copy link
Contributor

misund commented Jul 31, 2018

I realize this is a very vague comment. I'll come back with a stronger opinion tomorrow.

@misund misund self-assigned this Aug 1, 2018
Copy link
Contributor

@misund misund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's a good idea to specify exact sizes for each component in this way. If we apply this approach across the board, the number of settings would grow to a level that's not really maintainable. In short, this solution does not scale well.

Instead, we should calculate the margin and padding of components using horizontalBase and verticalBase.
If a brand has needs that this approach can't accomodate, we should consider creating another component.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants