spurious error for line-height

[wkeese]

Given the SCSS of

...
.expression {
    line-height: $spacing-07;
}

I'm getting the error that:

✖  Expected carbon type token, mixin or function to be set for variable "$spacing-07" used by "line-height" found "an unknown, undefined or unrecognized value".  carbon/type-token-use

The error message is spurious since I am using a carbon token. Or at least, misleading?

I agree that it's strange to set line-heightdirectly. In my case, I think it's justified. I am displaying expressions containing Tag components for variables, eg:

[lee-chase]

In cases you wish to override a rule then please disable with comment.

It is possible to redefine the list of properties being checked with includeProps: ["/^\\$my-color--/", "*"] but I think that is a bit cumbersome, not allowing removal currently of individual properties.

Raised https://github.com/carbon-design-system/stylelint-plugin-carbon-tokens/issues/118 to exclude props

.expression {
    /* stylelint-disable-next-line carbon/type-token-use */
    line-height: $spacing-07;
}

[wkeese]

@lee-chase - I don't want to override the rule. I just want it to work correctly. You agree that it's broken right?

[lee-chase]

@wkeese Happy to re-open this issue for further discussion, however I am not sure I regard it as wrong. The error message, which may not be clear, is reporting that a Carbon type token is expected when a Carbon layout token is being used.

This seems somewhat of an edge case that it would be perfectly reasonable use a disable with commentary. In general I do not think line-height should be set directly.

/* stylelint-disable-next-line carbon/type-token-use -- line-height to match tag size */

[wkeese]

Thanks @lee-chase.

The error message, which may not be clear, is reporting that a Carbon type token is expected when a Carbon layout token is being used.

I see. Hmm, what type tokens specifically would be appropriate for setting line-height?

I'm looking at @carbon/type and I see stuff about font sizes but nothing about line-height. Of course I also see the higher level mixins like $heading-01.

In general, I do not think line-height should be set directly.

I agree, although I think it makes sense in my case. If the error message said something to the effect that line-height/font-size/etc. shouldn't be set directly, I think it could be justified... but if that's the official policy then why does Carbon export mixins like carbon--font-weight()? It's sending a mixed message.

[lee-chase]

@wkeese the unofficial policy, or rather my opinion as the core Carbon team rarely comments on this package, is that line-height should be set as part of setting the font. The goal of the package is to encourage best behaviour where possible and permit use of exceptions through a disable.

With regards to setting the line height, are you doing this to adjust the vertical alignment of the text between the tags? If so you may be able to use vertical-align to achieve the same effect.

P.S. I also tend to have the following rules enabled, that last ensures every disable has a description.

  "reportNeedlessDisables": true,
  "reportInvalidScopeDisables": true,
  "reportDescriptionlessDisables": true,

[wkeese]

@wkeese the unofficial policy, or rather my opinion as the core Carbon team rarely comments on this package, is that line-height should be set as part of setting the font.

That's still a bit vague for me, but IIUC you are saying that this is recommended:

@include typography.type-style("body-long-01");

and that this is flagged:

line-height: ANYTHING

If that's the case, then I would suggest just changing the error message from:

✖ Expected carbon type token, mixin or function to be set for variable "$spacing-07" used by "line-height" found "an unknown, undefined or unrecognized value". carbon/type-token-use

to

✖ Line-height shouldn't be set directly. Use the typography.type-style() mixin instead. carbon/???

The current error message implies that you can set line-height to a valid or invalid value, but you haven't shown me any examples of valid values.

---

The goal of the package is to encourage best behaviour where possible and permit use of exceptions through a disable.

Sure, I agree with your goal.

---

With regards to setting the line height, are you doing this to adjust the vertical alignment of the text between the tags? If so you may be able to use vertical-align to achieve the same effect.

It's not about alignment, it's about making each line of text the same height. I want it to display like this:

rather than this:

It's a subtle difference but pay attention to the spacing between the first and second line compared to the spacing between the second and third line.

But as I said above, I understand this is a corner case, and that normally an app shouldn't set line-height directly. So I'm fine with throwing an error.

---

P.S. I also tend to have the following rules enabled, that last ensures every disable has a description.

👍 Me too, I hate mystery code without any comments.

[lee-chase]

Thanks for the feedback @wkeese I have raised #123 to address the generic message and will close this issue.