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

Ensure fieldset never exceeds max-width #1269

Merged
merged 1 commit into from
Apr 24, 2019

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Apr 5, 2019

This fix ensures that both WebKit/Blink and Firefox are prevented from expanding their fieldset widths to the content's minimum size.

Online demo
https://tourmaline-spoonbill.glitch.me/

This was preventing max-width: 100% from being applied to select menus inside a fieldset. There's further discussion in "Reset your fieldset" and raised by @kr8n3r in issue #1264.

Due to Firefox's legacy fieldset behaviour, I've also added a workaround for Firefox < 53.

// Fix for Firefox < 53
// https://bugzilla.mozilla.org/show_bug.cgi?id=504622
@supports not (caret-color: auto) {
  .govuk-fieldset,
  x:-moz-any-link {
    display: table-cell;
  }
}

It uses x:-moz-any-link to make the snippet Firefox-only, but with @supports not (caret-color: auto) to exclude Firefox >= 53 when caret-color support was added.

Firefox 52

@aliuk2012 aliuk2012 added the awaiting triage Needs triaging by team label Apr 8, 2019
@colinrotherham
Copy link
Contributor Author

Rebased with master

@kellylee-gds kellylee-gds removed the awaiting triage Needs triaging by team label Apr 10, 2019
CHANGELOG.md Outdated Show resolved Hide resolved
margin: 0;
padding: 0;
border: 0;
@include govuk-clearfix;
}

// Fix for Firefox < 53
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given Firefox 53 was released in April 2017 and we're now on version 66 I think we can probably omit this? On GOV.UK, 6.18% of Firefox users were using a version older than 53, which represents about 0.29% of the total traffic.

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@36degrees Honestly I'll leave this one down to you and the team. I was stung many times in my career by Firefox fieldset oddities so maybe I'm emotionally attached 😁

Copy link
Contributor Author

@colinrotherham colinrotherham Apr 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, whilst the default browser was changed to IE11 a few months ago, DWP Jobcentre work coaches and other staff still have access to Firefox 45 ESR.

It's included in various automated tests.

Is that reason to keep it? Thanks

Including workaround for Firefox < 53
@colinrotherham
Copy link
Contributor Author

@36degrees Just the last review point to address now: #1269 (comment)

Let me know if I should remove the Firefox < 53 fix.

Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @colinrotherham 👍

@aliuk2012 aliuk2012 merged commit fbb7cbf into alphagov:master Apr 24, 2019
36degrees added a commit that referenced this pull request Apr 25, 2019
This ended up in the release notes for v2.10, but it hasn't gone out yet
36degrees added a commit that referenced this pull request Apr 25, 2019
Move changelog entry for #1269 to correct place
@aliuk2012 aliuk2012 added this to the Next milestone Apr 25, 2019
@aliuk2012 aliuk2012 mentioned this pull request Apr 25, 2019
aliuk2012 added a commit to alphagov/govuk-prototype-kit that referenced this pull request Apr 25, 2019
- Add new secondary and warning button variants

  ([PR #1207](alphagov/govuk-frontend#1207))

- Add new govuk-shade and govuk-tint functions for creating shades and tints of
  colours.

  ([PR #1207](alphagov/govuk-frontend#1207))

- Add support for custom row classes on the summary list component (including support for some rows without action links)

  ([PR #1259](alphagov/govuk-frontend#1259))

- Ensure fieldset never exceeds max-width

  This fix ensures that both WebKit/Blink and Firefox are prevented from expanding their fieldset widths to the content's minimum size.

  This was preventing `max-width: 100%` from being applied to select menus inside a fieldset.

  See discussion in ["Reset your fieldset"](https://thatemil.com/blog/2015/01/03/reset-your-fieldset/) and raised by [issue #1264](alphagov/govuk-frontend#1264)

  ([PR #1269](alphagov/govuk-frontend#1269))

- Add various fixes to the summary list component:

  1. Fixes the 1px row height change when borders are removed
  Padding is now adjusted by 1px instead

  2. Fixes the text alignment when the actions column isn't added
  So the key column always stays at 30% width

  ([PR #1259](alphagov/govuk-frontend#1259))

https://github.com/alphagov/govuk-frontend/releases/tag/v2.11.0
@colinrotherham colinrotherham deleted the fieldset-select-overflow branch May 16, 2019 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

4 participants