-
Notifications
You must be signed in to change notification settings - Fork 328
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 settings associated with compatibility mode #2844
Conversation
/// | ||
/// @access public | ||
|
||
@mixin govuk-compatibility($product) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've generally not reviewed the content of the deprecation warnings – suggest it'd be a good idea to loop @claireashworth in on them, if you haven't already.
8ffb9d0
to
3715abc
Compare
I just realised I've changed all the warning text but didn't updated the associated tests 🤦🏻 Will sort out. |
796399e
to
3b1e449
Compare
Tests fixed, ready for re-review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technical implementation looks solid – thanks @owenatgov 🆒
I think some of the deprecation notices could be clearer, so I've made a some suggestions. I'd also personally prefer we use the full product name ('GOV.UK Frontend') where possible.
3b1e449
to
3d5df6d
Compare
To suppress lots of warnings when team members run this code locally due to parts of our own code currently using compatibility mode, we temporarily split `govuk-compatibility` into a public and private mixin. The private mixin is just `_govuk-compaitbility` as is and the public mixin references the private mixin but adds a warning. This is specifically to catch when users are using `govuk-compatibility` in their own code.
3d5df6d
to
2ccbba4
Compare
Fixes #2787
What/Why
Deprecates the public settings influenced by compatibility mode. This PR specifically targets instances when these settings are being used in user-authored sass outside the bounds of the compatibility mode variables. This scenario is unlikely but means we're covering all bases with our deprecation.
More details can be found in the original issue and descriptions of commits.