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

Remove legacy colour palette #3576

Merged
merged 5 commits into from
May 5, 2023
Merged

Conversation

querkmachine
Copy link
Member

@querkmachine querkmachine commented May 4, 2023

Part of removing compatibility mode.

Closes #2792.

Changes

Removing legacy colour palette

  • Removes $legacy parameter from instances of govuk-colour in components.
  • Removes separate legacy colour handling from Footer component and edit border-top style to account for removed variable.
  • Removes $govuk-use-legacy-palette and $govuk-colours settings.
  • Removes private $_govuk-colour-palette-legacy variable.
  • Renames $_govuk-colour-palette-modern to $govuk-colours, making it public and overrideable as per the original $govuk-colours setting.
  • Removes tests related to legacy colour palette.

Deprecations

We mistakenly did not deprecate the govuk-colour function's $legacy parameter at the same time as the related code in v4.4.0, so it's being deprecated in this release instead.

However, due to the removal of the other legacy colour palette code, it is non-operational and the parameter is only maintained to prevent compilation errors from outdated function references.

  • Updates Sassdoc for govuk-colour to note the deprecation.
  • Updates govuk-colour function to output a deprecation warning if the $legacy parameter is used.
  • Adds a test to ensure that the deprecation warning is being output.

@querkmachine querkmachine self-assigned this May 4, 2023
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3576 May 4, 2023 12:55 Inactive
@querkmachine querkmachine linked an issue May 4, 2023 that may be closed by this pull request
3 tasks
@querkmachine querkmachine requested a review from a team May 4, 2023 12:57

$_govuk-colour-palette-modern: (
$govuk-colours: (
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@colinrotherham colinrotherham self-requested a review May 4, 2023 13:08
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.

That was quick 🚀

I know we have #3575 in progress but will we follow up with the Review app changes?

Things like the ?legacy=true query handling:

app.use(middleware.legacy)

Plus all the href: "/" + legacyQuery Nunjucks wrangling etc

@querkmachine
Copy link
Member Author

@colinrotherham I imagine that's all going to be handled as part of #2794

@owenatgov
Copy link
Contributor

All looking good 👍🏻 A couple of comments:

  • To clarify the above comment, @querkmachine is right and part of that work will be to scrub legacy testing from the review app et al.
  • I'm a little nervous about the fact we're also removing the $legacy option from govuk-colour because we haven't explicitly deprecated it. This was an oversight on my part on in not deprecating it when I deprecated $govuk-use-legacy-palette. Practically speaking it doesn't do anything without the now removed setting but I'm worried we'll be breaking codebases for teams who may have it ingrained to do it as a best practice. I'm not gonna cling to this point. The functionality is fairly clearly tied to this removal and we attempt to be clear in our changelog that this is going away. Interested in thoughts on if this should stick around with a deprecation warning and be removed in v6 as a final layer of removal.

@querkmachine
Copy link
Member Author

Good point that we never explicitly deprecated $legacy, I'm not sure how best to handle that.

We could keep accepting the parameter and add a deprecation message, but given it won't actually work anymore, it doesn't strike me as technically being a deprecation, it's just a removal with fewer error messages.

Though given we have deprecated the $govuk-use-legacy-palette setting, whether activated manually or programatically via one of the Template/Toolkit/Elements settings, I don't think there's any way someone actually using the legacy palette wouldn't have seen a deprecation warning in 4.6.0. We might be able to get away with it?

Base automatically changed from remove-ie8-css to main May 4, 2023 14:04
@owenatgov
Copy link
Contributor

@querkmachine I've just found an example of it being used in govuk_publishing_components: https://github.com/alphagov/govuk_publishing_components/blob/99938773aef7a14e8f67c878f5ed0d9deddd09e0/app/assets/stylesheets/govuk_publishing_components/components/_layout-header.scss#L7 There are quite a few examples in gpc, I suspect from when they had to support legacy frameworks on some of their apps that consume this gem. 99% of govuk isn't using compatibility mode which is why I suspect this is primarily a best practice, however this will mean we have a scenario where teams will be using $legacy and may not have seen the warning. It feels to me like we have 2 options:

  1. Keep $legacy as a possible parameter to block unexpected errors that doesn't do anything besides throw up a warning telling users that this doesn't do anything and we'll be formally removing it in v6.
  2. Proceed with present course and get rid of it with a nod in our changelog.

I'm leaning towards 1 for the sake of code hygiene and us following a positive deprecation model, but I'm open to discussion.

@colinrotherham
Copy link
Contributor

Good practice for a v4 patch release to mop up any missed deprecations?

@querkmachine
Copy link
Member Author

I think a v4.6.x patch is probably a good shout. I can see the v4 releases still being used for a while, no matter how v5 development goes

  • If the v5.0.0 release is entirely about restructuring and removals without any significant new features, and it's subsequently released quite soon, then we may have a lot of teams hold off on it and stick to v4.6.x until there's more reason to upgrade.
  • If we do decide to incorporate bigger features into v5.0.0, then I can imagine development taking a while longer, in which circumstances more teams will have ended up on v4.6.x and see the new deprecations.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3576 May 5, 2023 14:27 Inactive
Copy link
Contributor

@owenatgov owenatgov left a comment

Choose a reason for hiding this comment

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

Adding a 2nd approval 🎨

@querkmachine querkmachine force-pushed the remove-compat-mode-legacy-colours branch from 146216d to 4fd51c6 Compare May 5, 2023 15:40
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3576 May 5, 2023 15:40 Inactive
@querkmachine querkmachine merged commit dc2d5a5 into main May 5, 2023
@querkmachine querkmachine deleted the remove-compat-mode-legacy-colours branch May 5, 2023 16:10
romaricpascal pushed a commit that referenced this pull request May 18, 2023
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Apr 3, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Apr 11, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Apr 12, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Apr 12, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Apr 12, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Apr 12, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Apr 15, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Apr 15, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Apr 16, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Apr 24, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request May 17, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request May 17, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request May 17, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request May 17, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request May 21, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request May 31, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Jun 4, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Jun 14, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Jun 24, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Jul 3, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Jul 5, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Jul 9, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Jul 11, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Jul 12, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
mike21573 added a commit to alphagov/signon that referenced this pull request Jul 12, 2024
The $legacy param was deprecated in the GOV.UK Frontend V5 release,
which our govuk_publishing_components gem is working on bundling soon.

See https://github.com/alphagov/govuk-frontend/releases/tag/v5.0.0 /
alphagov/govuk-frontend#3576
MartinJJones pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Jul 15, 2024
govuk-frontend v5 deprecates the `$legacy` param in the `govuk-colour`
mixin. Continuing to use it produces warnings for each instance of it
in the CSS. It can be safely removed as alphagov/govuk-frontend#3576
notes that `it is non-operational and the parameter is only maintained to
prevent compilation errors`.
mike21573 added a commit to alphagov/signon that referenced this pull request Jul 15, 2024
The $legacy param was deprecated in the GOV.UK Frontend V5 release,
which our govuk_publishing_components gem is working on bundling soon.

See https://github.com/alphagov/govuk-frontend/releases/tag/v5.0.0 /
alphagov/govuk-frontend#3576
mike21573 added a commit to alphagov/signon that referenced this pull request Jul 19, 2024
The $legacy param was deprecated in the GOV.UK Frontend V5 release,
which our govuk_publishing_components gem is working on bundling soon.

See https://github.com/alphagov/govuk-frontend/releases/tag/v5.0.0 /
alphagov/govuk-frontend#3576
mike21573 added a commit to alphagov/signon that referenced this pull request Jul 22, 2024
The $legacy param was deprecated in the GOV.UK Frontend V5 release,
which our govuk_publishing_components gem is working on bundling soon.

See https://github.com/alphagov/govuk-frontend/releases/tag/v5.0.0 /
alphagov/govuk-frontend#3576
mike21573 added a commit to alphagov/signon that referenced this pull request Jul 22, 2024
The $legacy param was deprecated in the GOV.UK Frontend V5 release,
which our govuk_publishing_components gem is working on bundling soon.

See https://github.com/alphagov/govuk-frontend/releases/tag/v5.0.0 /
alphagov/govuk-frontend#3576
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.

Remove legacy colour palette setting from codebase
5 participants