rsms / inter

The Inter font family
https://rsms.me/inter/
SIL Open Font License 1.1
17.68k stars 398 forks source link

The reported Scrollheight of an element is greater than its clientHeight #602

Closed tannerlyons closed 1 year ago

tannerlyons commented 1 year ago

Describe the bug It seems that the scrollHeight of an element is getting locked to ~1.2 * the font-size.

To Reproduce Steps to reproduce the behavior:

  1. See the reproduction here: https://codepen.io/tannermeans/pen/JjemjEM
    • That pen walks you through the repro. Note the scrollHeight reported there.

Expected behavior If an element has the following styles, I would expect it to NOT overflow i.e. the scrollHeight should be 16px;

display: inline; /* setting this to block will also reproduce the issue */
font-size: 16px;
line-height: 16px;
height: 16px;

Screenshots

This screenshot is from the codepen linked above and included for ease.

2023-07-26T00_07_1021x365_screenshot

Environment

Additional context

tannerlyons commented 1 year ago

Here's the same repro on Windows image (1)

tannerlyons commented 1 year ago

I've just confirmed it's the same on Mac OS Chrome.

acordy commented 1 year ago

@tannerlyons I don't believe this is an issue with Inter, or at least not one specific to it. I have tested the following fonts in your CodePen on MacOS Chrome: "Times", "SF Pro", as well as the generics sans-serif, serif, and monospace. They all yield scrollHeight values that differ from the explicitly set line-height. I'll note that Safari behaves differently, and does report the scrollHeight as matching the line-height.

From what I can tell, scrollHeight is not guaranteed to match the element's height. Furthermore, according the the spec (https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollHeight) it always returns an integer, so there are easily cases where an explicitly set line-height would be rounded up or down from the element's clientHeight, even if it did work as you're expecting.

I did some quick digging into the specifications on how scrollHeight is computed, and while it factors in the line-height, the specification includes considerations for the font's actual metrics, such as the ascenders and descenders. You can read more about this here:

As I mentioned earlier, there is some difference in what scrollHeight is reported between Chrome and Safari on MacOS, so it seems every user agent may handle it differently. In general I believe that what the specification is aiming for is to capture the value necessary to display the contents of the element. From MDN's docs:

The scrollHeight value is equal to the minimum height the element would require in order to fit all the content in the viewport without using a vertical scrollbar.

I don't think that it's a valid substitute for getting or setting an element's height (I imagine that this is what you're trying to use it for?)

In any case, the bottom line is that all the other fonts I tested behave this way, and that this is not an issue specific to Inter.

Hope this info helps.

tannerlyons commented 1 year ago

@acordy that's super helpful and I REALLY appreciate you taking the time out of your day to investigate so thoroughly. I learned a cool thing today. I will look over the links you provided and will suggest an alternative to my team.

FYI - the thing we were trying to do was to show a tooltip when content was elided vertically based on -webkit-line-clamp. We were comparing the offsetHeight to the scrollHeight to detect overflow.

Seems like we need to be really clear in our designs on how line-height can vary based on font metrics.

Again, thanks.

acordy commented 1 year ago

@tannerlyons Happy to help!

Based on your use case (as I understand it), you'll use -webkit-line-clamp to force a maximum number of lines for the tooltip, but my guess is that the issue for dev is that this could be lower than that max (as in you might set a max of 3 lines but sometimes it's 1 or 2).

Initial thought: Use the scrollHeight as the determinant, since it seems it's "generous" in how it computes the needed height, meaning it will return a value that's at least as large as needed for the tooltip text to be visible. Then maybe wrap the text on the element or its parent in a display: flex with everything centred (align-items: center and justify-content: center), which in my experience triggers the user agent to actually centre things.

Next thought: depending on your tech stack / framework, you can create the element, set the height to auto (an ancient approach), check its real height with something like offsetHeight or getBoundingClientRect().height and then on the next paint (setTimeout 0) set the height based on that.

Anyway, I do hope the info earlier has been useful, I'm sure you'll be able to resolve your issue!

rsms commented 1 year ago

@acordy thank you for the help!