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

Fix a couple of IE8 issues #737

Merged
merged 3 commits into from
May 31, 2018
Merged

Fix a couple of IE8 issues #737

merged 3 commits into from
May 31, 2018

Conversation

36degrees
Copy link
Contributor

@36degrees 36degrees commented May 30, 2018

Buttons

Fix missing button shadow by using a bottom-border in IE8.

Before

bs_winxp_ie_8 0 2

After

bs_winxp_ie_8 0 3

Breadcrumbs

Fix missing chevrons by falling back to any sans-serif font. IE8 seems to struggle to render pseudo-elements that use a font defined as a @font-face.

Before

bs_winxp_ie_8 0 4

After

bs_winxp_ie_8 0 5

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-737 May 30, 2018 15:26 Inactive

// IE8 doesn't seem to like rendering pseudo-elements using @font-faces,
// so fall back to using any sans-serif font to render the chevron.
font-family: sans-serif;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this safe? Browser do weird things, I wonder if picking something stable like helvetica/arial would be good to start with.

Copy link

@kr8n3r kr8n3r May 31, 2018

Choose a reason for hiding this comment

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

that would be forcing either a default instead of browser/os default combo

That said, our font stack definition is $govuk-nta-light: "nta", Arial, sans-serif;, so going by that we either do sans-serif or Arial, sans-serif

Not sure how up to date this table is https://www.granneman.com/webdev/coding/css/fonts-and-formatting/web-browser-font-defaults/

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, was thinking we use the current font stack minus nta

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use Arial, sans-serif.

IE8 doesn't seem to like rendering pseudo-elements using @font-faces, so fall back to using any sans-serif font to render the chevron.
IE8 does not support box-shadow, so use a border instead.

For browsers other than IE8 we use a transparent border to support users who override their browser colours, but the intersection of IE8 users who override their browser colours is likely to be negligible, so it should be OK to override this.
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-737 May 31, 2018 09:32 Inactive
Copy link
Contributor

@NickColley NickColley left a comment

Choose a reason for hiding this comment

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

Nice work!

The issue with chevrons was not present in the previous release (as the custom font-face has never worked in IE8 until this release) so does not need to be documented.
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.

4 participants