Typography noWrap hides high(vertical) characters

[albinekb]

• I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

When I use noWrap on Typography, it should not cut my "g" off, just like without noWrap

Current Behavior

Steps to Reproduce (for bugs)

Use https://material-ui-1dab0.firebaseapp.com/demos/drawers/#responsive-drawer and add a g
in the title

Context

This is happening because adding noWrap adds overflow: hidden, is this needed? Can we switch to overflow-x: hidden or remove it completely?

Another fix would be to make the component with overflow: hidden fit higher chars.

Thoughts?

Your Environment

Tech	Version
Material-UI	latest
React	16.0.0
browser	chrome canary

[oliviertassinari]

Can we switch to overflow-x: hidden or remove it completely?

I can't reproduce the issue, but using overflow-x: hidden sounds like a good idea (we need to keep it)

[JeromeFitz]

I can swap this to overflow-x: hidden. 👍️

[JeromeFitz]

That broke via argos however adding (while keeping overflow: 'hidden':

• lineHeight: 'inherit'

Has it showing appropriately.

Before (as mentioned by @albinekb):

After:

However, it does move up the main content a tinch.

I am going to add this to the PR to see how argos handles this (since I cannot duplicate the scrollbars).

[JeromeFitz]

The tinch up breaks argos (as it should). I will try a few more combos and see if we can get desired behavior while keeping existing.

[JeromeFitz]

When we add overflowX: 'hidden' it looks to set overflowY: 'auto'. So without a hack, that will not work (this looks to be per CSS3 Spec).

When we put lineHeight: 'inherit' on the core Typography noWrap element, this works with keeping overflow: 'hidden' while having no scrollbars, the desired ... effect, and no cut-offs on -g for Title.

<Typography type="title" color="inherit" noWrap>

However, in the examples for:

• MiniDrawer
• PermanentDrawer
• PersistentDrawer
• ResponsiveDrawer

In their content, they are also using:

<Typography type="body1" noWrap>

Which is causing the display shift in argos. Since in these examples the height, marginTop is handled in the demos themselves.

Note: AppFrame is using the Typography Component with noWrap with its type of title - this looks to be the only other place.

So we could:

1. Keep noWrap on content in the demos: Change the classes.content to adjust the height, marginTop in its root and for [theme.breakpoints.up('sm')].

• This would keep argos passing without much trouble.

2. Remove noWrap on content in the examples.

• This would bring the text back up to its expected height (via argos) but break its eventual end-state due to the new line (causing a new run of expectations in argos).

3. Allow for new adjusted behavior in argos in how noWrap works going forward with only a change to Typography.noWrap.

4. Go back to the drawing board. 🤣️

I can make the changes to move forward however you see fit.

Here are some screenshots:

Current:

Result of 1 before margin adjustment:

Result of 1 after margin adjustment:

Result of 2 with noWrap removed:

[JeromeFitz]

Went with Option 1 in the above comment to see if we could use argos in its current style regression state.

Bumped up the content container pixels by 2 where appropriate for the drawers and everything is passing the latest PR. 🖼️

[oliviertassinari]

@JeromeFitz Thanks for pushing it forward!

After looking more into the issue, I confirm it. Maybe we shouldn't be using line-height: 1 in the first place. This StackOverflow answer is interesting. As we can see in the CSS spec:

The font size corresponds to the em square, a concept used in typography. Note that certain glyphs may bleed outside their em squares.

material-components-web is using some different values. Still, their display 4 and 3 are affected by the same issue.

So, I would suggest using line-height: 1.11 instead of line-height: 1 as the minimal line-height value.

But then adding the following style background: transparent url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAIAAAAICAYAAADTLS5CAAAAFklEQVQYV2NkgAJG8hjl5eX/Ozs7GQEVHAQJMG4X6QAAAABJRU5ErkJggg==); } show that we have some Vertical Rhythm issue:

It's something we can compare against the spec:

(they are using a Print design Baseline grid as opposed to the Web one).
Here is a great article on the Vertical Rhythm topic.

[JeromeFitz]

@oliviertassinari Thank you as always for such detail. This is incredibly helpful not just from this issue level, but the entirety of your project scope.

The Vertical Rhythm topic is a great read. I've been making some adjustments to a local branch with the comments you shared (lineHeight, material-web-components, etc.) , however, before making any PR's going forward I will come back here with questions.

Though, as I was writing, I did just see you self-assigned this. I can stand down as well if the extra cook in the kitchen is not needed and would be more work on your end. Thanks again!

[JeromeFitz]

@albinekb, a (possible) short-term solution on your end would be to add the lineHeight directly to your element for now. You may need to tweak to fit your application:

<Typography type="title" color="inherit" style={{ lineHeight: 'inherit' }} noWrap>
  Inkommande bokningar
</Typography>

[albinekb]

Thanks, that's totally acceptable for me 👍  @JeromeFitz
And just WOW to the amount of effort and knowledge everyone put into this library, it's just amazing. I love it, it's so easy and straightforward to use. Which makes it a lot of fun 👍

[JeromeFitz]

Awesome!

However, all the credit goes to @oliviertassinari ❤️ 😀️ 👍️