carbon-design-system / stylelint-plugin-carbon-tokens

Apache License 2.0
9 stars 3 forks source link

Do not warn on font-style #105

Closed tay1orjones closed 4 months ago

tay1orjones commented 10 months ago

Echoing discussion on this issue, the linter shouldn't warn on direct usages of font-style. There's no way to configure this via a token and the only available options are normal and italic anyway. I think it's reasonable to modify the linter rule to not warn on usages of font-style.

lee-chase commented 4 months ago

Added option to accept font style acceptCarbonFontStyleFunction and an issue to review defaults before publishing v3.

https://github.com/carbon-design-system/stylelint-plugin-carbon-tokens/issues/119

lee-chase commented 4 months ago

Fixed in 2.4.0

wkeese commented 4 months ago

Is there any case where a user would want to set that acceptCarbonFontStyleFunction option to false?

lee-chase commented 4 months ago

@wkeese I will be reviewing the defaults and options for V3, for V2 I added this option to allow users to continue to get the same results.

wkeese commented 4 months ago

I think you're overthinking it. All bug fixes change the results.

And even if there were a backward-compatibility issue with V2, I don't see any reason for V3 to have an acceptCarbonFontStyleFunction option.

The goal here (as explained in #15131) is to not flag this CSS:

font-style: italic

Notice that it has nothing to do with a "font style function".

But even if you renamed the option to acceptDirectFontStyle or something like that, what is the value of letting the user toggle that option?

The only justification I can think of is to allow/disallow directly setting font-style, font-weight, line-height etc., rather than using CSS like @include type.type-style("productive-heading-01").

But in that case, the flag should be called something different (and do something different).

lee-chase commented 4 months ago

Reopening and considering options for v3

lee-chase commented 4 months ago

@wkeese should no longer warn from 2.8.0 onwards