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

Simplify legacy IE logic #631

Merged
merged 9 commits into from
Apr 5, 2018
Merged

Simplify legacy IE logic #631

merged 9 commits into from
Apr 5, 2018

Conversation

36degrees
Copy link
Contributor

@36degrees 36degrees commented Apr 5, 2018

  • Simplify the legacy IE logic now that we only need to support targeting IE8.
  • Remove anything targeting IE6 or IE7.
  • Remove the requirement for users to set $mq-responsive: false; in their IE8 stylesheet by setting this value based on $govuk-is-ie8.

Recommend reviewing commit by commit.

I have validated that this change does not affect the generated CSS by running npm run build:dist on both the master branch and this branch, copying the generated files from dist/css and comparing the files:

md5 **/*.css
MD5 (master/govuk-frontend-0.0.26-alpha.min.css) = 9515f244a0a79d1e7b307d0d696c3633
MD5 (master/govuk-frontend-old-ie-0.0.26-alpha.min.css) = 3dfa4089277139b33decb1af6512d126
MD5 (simplify-legacy-ie-logic/govuk-frontend-0.0.26-alpha.min.css) = 9515f244a0a79d1e7b307d0d696c3633
MD5 (simplify-legacy-ie-logic/govuk-frontend-old-ie-0.0.26-alpha.min.css) = 3dfa4089277139b33decb1af6512d126

GOV.UK Frontend will not support IE6 or IE7 - we don’t test with those browsers, and we don’t provide any guidance on building stylesheets to target them. Removing these rules simplifies the codebase and allows us to simplify the ‘legacy IE’ logic by removing reference to versions.
Removing the $version check and updating the mixins to be specific to IE8 makes them easier to use and simplifies the codebase.
For consistency, introduce a negated helper that allows rules to be excluded when compiling to target IE8, rather than the current approach of referring to the setting directly.
Rather than relying on the user remembering to specify another setting when building a stylesheet that targets IE8, modify the media queries settings to use sensible defaults based on the value of `$govuk-is-ie8`.

This means we need to ‘promote’ the `$govuk-is-ie8` variable to the settings layer so that we can include it before the media queries settings.
@36degrees 36degrees changed the title Simplify legacy ie logic Simplify legacy IE logic Apr 5, 2018
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-631 April 5, 2018 09:04 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-631 April 5, 2018 12:56 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.

Seems like a sensible refactor, nice work 👍

@alex-ju
Copy link
Contributor

alex-ju commented Apr 16, 2018

Should we update legacy-ie.md based on these changes?

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