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

Fieldset legend text can be clipped when using a custom or fallback font #1239

Closed
stevesims opened this issue Mar 5, 2019 · 7 comments · Fixed by #1548
Closed

Fieldset legend text can be clipped when using a custom or fallback font #1239

stevesims opened this issue Mar 5, 2019 · 7 comments · Fixed by #1548
Assignees
Labels
accessibility accessibility-regulations-failure Does not meet the Public Sector Accessibility Regulations (WCAG 2.1 level AA) 🕔 hours A well understood issue which we expect to take less than a day to resolve.

Comments

@stevesims
Copy link

The SASS for .govuk-fieldset__legend has a line that sets overflow: hidden, as follows:

// Hack to let legends or elements within legends have margins in webkit browsers

From what I can tell the webkit bug this relates to was this one which was fixed over 6 years ago.

Having this in there can mean that if the NTA font isn't present, the bottom of legends can get chopped off for larger font sizes, and the descenders in Arial end up overflowing the element and thus hidden.

@NickColley
Copy link
Contributor

NickColley commented Mar 5, 2019

Hey @stevesims, thanks for letting us know about this.

I have reproduced this here:

https://govuk-frontend-issue-1239.glitch.me/

You can see the descenders from the 'g' in the title is clipped.

screen shot 2019-03-05 at 17 48 05

screen shot 2019-03-05 at 17 53 15

You can see that with the regular headings that they also clip, but because they do not have this 'overflow: hidden' you will not notice it.

screen shot 2019-03-05 at 17 54 30

I think this is likely because the font scale has been designed with NTA only in mind.

We have a new version of the font that has a different baseline which might avoid this issue.

@dashouse might have some insight into what is the best thing to do here.

@NickColley NickColley changed the title Webkit hack for margins in fieldset legends should be removed Fieldset legend text can be clipped when using a custom or fallback font Mar 5, 2019
@NickColley
Copy link
Contributor

I've updated the title to make it focused on the problem as there may be a different solution to this issue, hope this is OK

@NickColley NickColley added the awaiting triage Needs triaging by team label Mar 5, 2019
@dashouse
Copy link

dashouse commented Mar 6, 2019

Good spot, we did a fair bit of checking between NTA and Arial but the legend specific styles were added a couple of months after that so didn't spot this.

The fact it's overflowing generally is ok, I believe it's because the line-height for the larger font sizes is actually smaller than the em-box of the font so they technically overlap even though the actual glyphs don't. This is to have a computed line height of 50px to lock it in the the baseline grid as well as feeling about right.

The newer cut of the font sits in the same position as Arial / Helvetica so we'll have to test this.

Do we know what versions of browsers would be affected by removing overflow: hidden?

@36degrees
Copy link
Contributor

The 'hack' was introduced in alphagov/govuk_elements#484 in June 2017, so I don't think it relates to the bug you reference given how old it is.

From reading that PR, I think it's a fix to allow hint text within legends to have margins.

As we don't put hints or error messages within the legends any more perhaps it's not be a problem any more, and the hack can be removed? We'd need to test it thoroughly in all of our supported browsers.

@stevesims
Copy link
Author

I just read around a bit more on this. I can't find much by googling about that's "current" (within the past 3 years). The references on alphagov/govuk_elements#484 point towards a rejected PR alphagov/govuk_elements#76 which contains the comment:

In Webkit, the bottom margin of a legend and the top margin of the next element don't overlap, which they should. This addresses that by discarding the top margin of the next element

This may well be the underlying issue that was attempted to be addressed. (Again tho, I'm not sure whether this would still be an existant issue in WebKit/Blink based browsers - would need to experiment/test to find out.)

Given that spacing between elements in govuk-frontend primarily (with only a few exceptions) works through the use of margin-bottom it does feel that this would no longer be an issue.

@kellylee-gds kellylee-gds added 🕔 hours A well understood issue which we expect to take less than a day to resolve. Priority: Low and removed awaiting triage Needs triaging by team labels Mar 6, 2019
@kellylee-gds kellylee-gds added this to Icebox in Design System Sprint Board via automation Mar 6, 2019
@timpaul timpaul moved this from Icebox to Backlog in Design System Sprint Board Mar 11, 2019
@kellylee-gds kellylee-gds moved this from Backlog to Icebox in Design System Sprint Board Apr 30, 2019
@andymantell
Copy link
Contributor

andymantell commented Aug 22, 2019

Just to say that I have observed this with the NTA font as well, it doesn't just seem to occur with Arial or other fallbacks. As alluded to above, I suspect this has now got worse with the new cut of the font. See demo here:

https://bush-monarch.glitch.me/

And screenshot of the above:

image

@hannalaakso hannalaakso added awaiting triage Needs triaging by team and removed 🕔 hours A well understood issue which we expect to take less than a day to resolve. Priority: low labels Aug 22, 2019
@kellylee-gds kellylee-gds added 🕔 hours A well understood issue which we expect to take less than a day to resolve. Priority: low accessibility and removed awaiting triage Needs triaging by team labels Aug 28, 2019
@NickColley NickColley self-assigned this Sep 2, 2019
@NickColley NickColley added this to Upcoming sprints in Design System Sprint Board via automation Sep 2, 2019
@NickColley
Copy link
Contributor

Note that your example Andy has overriden font (since it's not on a gov.uk domain), so the demonstration Glitch apps use a custom font.

Here's what happens if you don't use a custom font:

font is not clipped

@NickColley NickColley moved this from Needs review to Done in Design System Sprint Board Sep 3, 2019
@NickColley NickColley added the accessibility-regulations-failure Does not meet the Public Sector Accessibility Regulations (WCAG 2.1 level AA) label Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility accessibility-regulations-failure Does not meet the Public Sector Accessibility Regulations (WCAG 2.1 level AA) 🕔 hours A well understood issue which we expect to take less than a day to resolve.
Projects
Development

Successfully merging a pull request may close this issue.

7 participants