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

Deprecate IE8 related settings and tools #3343

Merged
merged 4 commits into from
Feb 28, 2023
Merged

Conversation

36degrees
Copy link
Contributor

We'll be removing IE8 support in v5.0. Following our guidance for deprecating and removing features, deprecate the Sass settings and mixins used for building IE8 specific stylesheets:

  • the $govuk-is-ie8 setting
  • the $govuk-ie8-breakpoint setting
  • the govuk-if-ie8 mixin
  • the govuk-not-ie8 mixin

Update references to the mixins within GOV.UK Frontend so that they do not generate warnings – we only want warnings to be generated where service teams are using the govuk-if-ie8 and govuk-not-ie8 mixins in their own code.

Suppress IE8 related warnings when generating the IE8 stylesheets for the review app or for the dist version of GOV.UK Frontend.

Deprecating these features communicates this intent to service teams and allows them to make changes ahead of time, reducing the effort involved in upgrading to the next major version.

Closes #3163

Update internal references to the `govuk-if-ie8` and `govuk-not-ie8` mixins to call the new private versions of the mixins which do not trigger the warnings.
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3343 February 27, 2023 16:45 Inactive
@colinrotherham colinrotherham self-requested a review February 27, 2023 18:09
Copy link
Contributor

@colinrotherham colinrotherham left a comment

Choose a reason for hiding this comment

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

🎉

@colinrotherham
Copy link
Contributor

Looks brill. Noticed a fair few of them in:

Just mentioning #3164 too as it'll need the new ie8.test.js test adjusting

Note: Not necessarily related, but all the Sass "tooling" tests spin up the Review app as they're named to match the "JavaScript behaviour tests" project. Annoying you could use ie8.test.js ie8.unit.test.js but that sets up JSDOM 🤷‍♂️

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3343 February 28, 2023 14:33 Inactive
@36degrees
Copy link
Contributor Author

@claireashworth I've added a changelog entry for this, but it feels a little clunky (especially the title) – would you like to take a look now or happy to take a look when we come to do the release?

@claireashworth
Copy link
Contributor

claireashworth commented Feb 28, 2023 via email

CHANGELOG.md Outdated
@@ -43,6 +43,19 @@ Disabling links that are styled to look like buttons will not be supported by fu

This was added in [pull request #3326: Deprecate `govuk-button--disabled` class](https://github.com/alphagov/govuk-frontend/pull/3326).

#### Stop using the `govuk-if-ie8` and `govuk-not-ie8` mixins and the `$govuk-is-ie8` and `$govuk-ie8-breakpoint` settings
Copy link
Contributor

Choose a reason for hiding this comment

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

Alternative headings:
Stop using the mixins govuk-if-ie8 and govuk-not-ie8 and the breakpoint settings $govuk-is-ie8 and $govuk-ie8-breakpoint
or
Stop using the mixins and breakpoint settings for generating IE8 stylesheets
or
Stop using the deprecated IE8 mixins and settings.

It depends how much detail you want in the header.

Copy link
Contributor

Choose a reason for hiding this comment

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

@36degrees I've made some suggestions, but the rest looks fine to me.

@36degrees 36degrees force-pushed the deprecate-ie8-vars-mixins branch from 7b327e3 to 512fbdb Compare February 28, 2023 17:38
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3343 February 28, 2023 17:38 Inactive
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.

Deprecate Sass settings and mixins for building IE8 specific stylesheets
4 participants