Make set-type margins optional

[khawkins98]

There are cases where we want to use the default typography, but not have margins.

vf-badge is a good example of this:

The fix here is one approach, a few other options:

1. As I've done but with different variable names/logic (not sure if I've done a good approach)"

• mixin set-type($font-size, $global-font-family: $global-font-family, $set-margins: true) {

2. Just let the margin: 0 0 var(--vf-text-margin--bottom, 16px) 0; and then unset/add margin-bottom: 0 (feels messy)
3. Update the token YAML with a new prop:

  - name: button--r
    value:
      font-size: 16px
      font-weight: 700
      line-height: 1
      margin: 0 0 var(--vf-text-margin--bottom, 16px) 0

The way I've gone in this PR feels like the most balanced approach, I think.

[sturobson]

As the components seeemingly have different margins, and the variants could also have different margins. I wonder if we should leave it to a component level to set margin, instead of abstracting it away?

[khawkins98]

I do like that the set-type gives you a reasonable default. I think we could also make it a bit more reasonable by looking at the font setting being passed and setting different margins...

• body--* could get one setting
• heading--* another
• button--* gets nothing
• and then an optional component param to disable the margins

(See updated PR)

[sturobson]

I like this, do you think it'd would be even better if we allow $margin to have a value that's passed through?

I'm wondering as we're using CSS custom properties that we can reset it, as needed, at the component level.

[sturobson]

Do we need to set an @if for the zero margins?

[khawkins98]

Only thing I'd worry about doing --vf-text-margin--bottom: 0; at the component level: could potentially cascade within the component in weird ways? In theory, we should be able to scope our way out of it...

But, updated the PR with your thoughts, so now it supports three states: passed variable, automatic/default, disabled.

[sturobson]

cool :)

the idea is being employed in vf-summary where changes are scoped to the component variant:

.vf-summary--news .vf-summary__date {
  --vf-text-margin--bottom: 4px
}

[khawkins98]

Sounds good. I'm more than happy to defer to your judgement on how we should be setting/passing things around in the code, you've far more experience with the design/maintenance of 'em.