Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename leading-* to lh-* and use numeric scale #664

Closed
wants to merge 1 commit into from

Conversation

adamwathan
Copy link
Member

@adamwathan adamwathan commented Feb 18, 2019

This PR changes the name of our line-height classes from leading-* to lh-*.

Old Class New Class
leading-none lh-1
leading-tight lh-1.25
leading-normal lh-1.5
leading-loose lh-2

Changing the class prefix

The primary motivation for this is to make the naming mirror the CSS property more closely, as leading is an unfamiliar term to many. I considered line-height-* and line-h-* as well, but based on a Twitter poll I ran, people preferred the lh-* abbreviation (and so do I).

I noticed GitHub uses this prefix in Primer which also made me feel more comfortable with it, since in general they tend to use more verbose names than Tailwind.

Switching to a numeric scale

One of the most annoying things about leading-normal to me is that on almost every project I work on, I set the default line-height to leading-tight right on the <body> tag. So my "normal" line-height is tight, and leading-normal isn't normal at all. I could change the value of leading-normal to be 1.25 on those projects, but then what name should I use for 1.5?

On top of that, it's hard to introduce new values in the existing descriptive scale. Switching to a literal numeric scale makes that trivial.

In general, I don't think the descriptive scale is a useful abstraction in this case. Users can always add descriptive value if they like, like lh-copy, lh-heading, etc. but I think starting with literal values serves people better by default with no real downside.

Changing the core plugin name

This PR also changes the core plugin name from leading to lineHeight, so line-height values would be customized by editing the lineHeight key in the theme section of your config, and variants would be customized by editing the lineHeight key in the variants section of your config. This is in line with #656 and along with the class name change, and makes these settings more easily guessable.

@adamwathan adamwathan changed the title Change leading-* to lh-* and use numeric scale Rename leading-* to lh-* and use numeric scale Feb 18, 2019
@aparajita
Copy link

If you're going to use a numeric scale, please don't use a scale that tracks directly to the default values. In one of my projects, lh-1.5 has a value of 1.4. So I would end up having to change both the name and the value to prevent confusion. Not a great DX.

Just a thought.

@jackmcdade
Copy link
Contributor

jackmcdade commented Feb 18, 2019

I honestly never had an issue with a single class name, so I personally don't see the benefit in breaking changes. That being said, decimals in class names looks awkward and I would love to avoid that. I've never needed more than 3 line height sizes in any site i've ever done, so normal, loose, and tight is more than adequate, and fantastically descriptive.

Additionally, whenever you change a font, and line-height change can be necessary,I don't look forward to having to update lg-1.5 to lg-1.6 everywhere when that happens because I switched from Helvetica to Inter UI.

You can always go with relative modifiers. tight, tighter, tightest, etc.

@dillingham
Copy link

dillingham commented Feb 18, 2019

Still in favor of “line” over “lh”
Especially with the “letter” PR
And possibly “word” in the future
It would be clean if they shared a convention

property tailwind
letter-spacing letter-1
line-height line-3.5
word-spacing word-2

Singular is better #664 (comment)

@sandren
Copy link

sandren commented Feb 18, 2019

I think the reduction of ambiguity is worth the introduction of the decimal.

@jackmcdade
Copy link
Contributor

jackmcdade commented Feb 18, 2019

I think the reduction of ambiguity is worth the introduction of the decimal.

@sandren What if you need to change your line-heigh thought? Do you want to find & replace all occurrences of 1.5 with 1.75 or whatever?

@adamwathan
Copy link
Member Author

My big problem is the one I outlined in the original message, which is that leading-normal is never my normal line height, but maybe I just need to get over that.

@jackmcdade
Copy link
Contributor

jackmcdade commented Feb 18, 2019

My big problem is the one I outlined in the original message, which is that leading-normal is never my normal line height, but maybe I just need to get over that.

@adamwathan Just change the default values. I do. To me, none is not 1, none is 0. I do that on all projects. 🤷‍♂️

@adamwathan
Copy link
Member Author

What if you need to change your line-height thought? Do you want to find & replace all occurrences of 1.5 with 1.75 or whatever?

@jackmcdade Nothing stopping you from using descriptive names even if the defaults are numeric though right? I still want the defaults to be as good as we can make them and agree the defaults nudge people in a certain direction though.

For some reason I don't love lh-tight though, I like leading-tight better. But I really want the actual plugin names to be close to the CSS properties, and it would be really weird to have to edit the lineHeight key to change the leading values. I dunno, feels like I can't win on this one 😩

Just change the default values. I do. None is not 1, none is 0.

What name do you use for 1?

@zaknesler
Copy link

zaknesler commented Feb 18, 2019

#EmmetLivesMatter

😃 .bg-blue.text-white.lh-2.5

😃 [tab]

😢 <div class="bg-blue text-white lh-2 5"></div>

@sandren
Copy link

sandren commented Feb 18, 2019

My big problem is the one I outlined in the original message, which is that leading-normal is never my normal line height, but maybe I just need to get over that.

I think this is a common issue as what is considered "normal" leading is unique to each project and even each typeface within that project. It seems that avoiding this change only forces customization, which is something we are trying to avoid in the v1.0 release (see changes to the configuration).

@jackmcdade
Copy link
Contributor

Nothing stopping you from using descriptive names even if the defaults are numeric though right? I
@adamwathan True, and fair. Just giving you the requested feedback. People's first experience with something dictates how they feel about it forever. My first experience was joy and happiness with no friction or frustration, so why mess with it?

What name do you use for 1?

I rarely use 1, it almost never looks right to me. :) But if I did, I would use tightest.

@benface
Copy link
Contributor

benface commented Feb 18, 2019

@jackmcdade A line-height of 0 results in overlapping lines of text. I think none in terms of leading/line-height means "no extra space between the lines".

I'm really undecided on this one. I am not a fan of lh, and not a fan of dots in class names either, but thanks to Tailwind I find colons in class names completely normal now, so I'll get used to anything. I kinda agree with @dillingham that it feels more consistent to go with lines- if we're going to have letters-...

@adamwathan
Copy link
Member Author

#EmmetLivesMatter

@zaknesler Tailwind already sucks with Emmet because of the variant classes (hover:bg-red) and the fractional widths (w-1/2) so not too bothered by that.

@jackmcdade
Copy link
Contributor

It seems that avoiding this change only forces customization, which is something we are trying to avoid in the v1.0 release (see changes to the configuration).

Ah, I didn't realize this was a new goal. I guess it makes a little bit more sense then.

@adamwathan
Copy link
Member Author

Ah, I didn't realize this was a new goal. I guess it makes a little bit more sense then.

I would say the goal is just to make the defaults better suited for more projects, because lots of people already don't customize. Picking bad defaults that force people to always customize is definitely something I want to avoid, but I don't think any of the ideas in this thread really fall under that category, they all have merit.

@adamwathan
Copy link
Member Author

I kinda agree with @dillingham that it feels more consistent to go with lines- if we're going to have letters-...

@benface The only thing I don't like about lines-* is that I'm very frequently setting the line-height on an element that will never have wrapping words, so there will never be multiple lines.

<button class="lines-tight">...</button>

...just doesn't feel as correct as saying "set the height of a single line to x" – it feels like I'm saying "set the space between lines to x", and if there are never multiple lines then it feels like I'm setting something over here because of a coincidental effect it has on this thing over there, you know?

@benface
Copy link
Contributor

benface commented Feb 18, 2019

Totally. That's a good point.

@benface
Copy link
Contributor

benface commented Feb 18, 2019

But the words "tight" and "loose" also refer to the spacing between lines rather than the height of a line.

@dillingham
Copy link

dillingham commented Feb 18, 2019

I think that’s why it’s better singular
And probably why line-height isn’t plural

<button class=“line-3 letter-2”>

@sandren
Copy link

sandren commented Feb 18, 2019

But the words "tight" and "loose" also refer to the spacing between lines rather than the height of a line.

All the more reason to support the merge. 😅

@adamwathan
Copy link
Member Author

But the words "tight" and "loose" also refer to the spacing between lines rather than the height of a line.

Yeah, that's kinda why I like the numbers, but I don't hate the descriptive terms either, I definitely prefer lh-tight to lines-tight, but prefer leading-tight to either of them I think.

I just hate the idea of having a lineHeight plugin that uses the class name leading because it's pretty unintuitive to have to edit the lineHeight section in your config to add additional leading values. But naming the plugin leading also breaks the plugin naming convention.

@jackmcdade
Copy link
Contributor

You're never going to get the name, the term, and the plugin convention perfect because the css spec is a mess. Go with your gut and ship it.

@adamwathan
Copy link
Member Author

You're never going to get the name, the term, and the plugin convention perfect because the css spec is a mess. Go with your gut and ship it.

I WILL GET IT PERFECT IF IT KILLS ME JACK.

@avvertix
Copy link

avvertix commented Feb 18, 2019

So my "normal" line-height is tight, and leading-normal isn't normal at all. I could change the value of leading-normal to be 1.25 on those projects, but then what name should I use for 1.5?

What about following the same naming as the font sizes

Old Class New Class
leading-none lh-base
leading-tight lh-lg
leading-normal lh-xl
leading-loose lh-2xl

I would not trigger a big discussion, but going for lh to be close to line-height, would mean that text-sm (for referring to font-size) will be fs-sm?

I considered line-height-* and line-h-* as well, but based on a Twitter poll

I would have been more prone to use line-, which to me is close to the CSS, but also simple enough to remember

@hacknug
Copy link
Contributor

hacknug commented Feb 18, 2019

I hate to be that guy but imo leading is a better fit than lines or lh… I'm also completely biased because I am a designer and even if I didn't study in English I've known the terms for a long time. lh is okay but only if tracking becomes ls I guess.

Here's the thing: If we're okay with getting rid of almost everything in the config file because users can look things up on the docs, do we really need to change this?

Is this a real issue that we need to solve? Because if it's not I'd just leave the way it is seeing how many opinions everyone has about it (second most commented PR in the repo, third most commented post if we're counting issues) and how even Adam looks unsure this is needed.

Maybe core plugins should be named after the CSS property or module they're related to and the class name they output be a completely different thing? It wouldn't be that bad now that users are going to (almost?) be able to configure their own class names.

@aparajita
Copy link

@adamwathan You'll never please everyone. I hope you see the point I made on twitter about allowing the names to be configurable. There is such a wide variety of opinions on naming, why not let the user easily change the naming to what suits them? That doesn't take anything away from an awesome set of defaults that many people will use as is.

@dillingham
Copy link

dillingham commented Feb 18, 2019

Custom names is a bit overkill when most are voicing opinions only because they are asked, but that doesn’t warrant a need for custom names just because people are pitching ideas

and being a designer doesn’t help the many of developers who aren’t, I think it’s wise and painless to move away from print concepts for mass appeal for those who use css to now use utility css

@adamwathan adamwathan changed the title Rename leading-* to lh-* and use numeric scale Rename leading-* to lh-* and use numeric scale Feb 18, 2019
@aparajita
Copy link

Custom names is a bit overkill to change lh to line when most are voicing opinions only because they are asked

The change is from leading to lh. Big change. People will adopt whatever they're given because they have no choice unless they want to write a plugin themselves.

@itmayziii
Copy link

itmayziii commented Feb 19, 2019

Please go with the numeric scale as opposed to something like tight, tightest, loose

The problem with these non numeric descriptors is that they leave no wiggle room, if I need a value between tight and tightest then I have to resort to adding a tighter class or worse if I need another value in between then I have to do tight-tighter which does not read well. I know it can be weird to see decimals in variable names but something like lh-1.3 has a lot more value when reading it as a class than lh-tight-tighter.

I'm also going to note I feel the same way about t-shirt sizes, lh-lg and lh-xl are fine until you need a value in between then your left with lh-lg-xl. Not only is this a rabbit hole of coming up with clever names but lg-xl doesn't mean anything to me. I really like your quarter based sizes approach to some of the classes p-8 is padding on all sides at 2rems because 8 quarters = 2.

@sandren
Copy link

sandren commented Feb 19, 2019

and being a designer doesn’t help the many of developers who aren’t, I think it’s wise and painless to move away from print concepts for mass appeal for those who use css to now use utility css

But they're not print concepts. They are typographical concepts and the terms are completely appropriate for the web.

@jsblair9
Copy link

jsblair9 commented Feb 19, 2019

I think I agree with @jackmcdade on this one. I usually use this in my config for leading:

leading: {
  'zero'  : 0,
  'none'  : 1,
  'tight' : 1.25,
  'normal': 1.5,
  'loose' : 2,
}

And often have .leading-tight set on the body since it is what usually feels normal

@dillingham
Copy link

dillingham commented Feb 19, 2019

@sandren sorry, replace “print” with “design”

If you can’t see my point it’s cause you’re a designer :) not saying it’s wrong, I’m saying it’s not intuitive to people who write css without being a designer.

@sandren
Copy link

sandren commented Feb 19, 2019

If you can’t see my point it’s cause you’re a designer :)

Guilty as charged. 😅

@adamwathan
Copy link
Member Author

Decided to just leave everything as is, more here: #667 (comment)

No point breaking people's sites for a class name change that I'm not even sure is actually better.

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

Successfully merging this pull request may close these issues.

None yet