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 ordering of properties in static spacing override classes #2770

Merged
merged 1 commit into from
Aug 17, 2022

Conversation

vanitabarrett
Copy link
Contributor

We incorrectly shipped static spacing override classes in the format
govuk-!-padding-static and govuk-!-margin-static, when we intended
them to be in the format govuk-!-static.

Our documentation refers to govuk-!static. We should update the classes to match,
but keep the existing classes for now as people may already be using them.

@vanitabarrett vanitabarrett changed the title Fix ordering of properties in spacing override classes Fix ordering of properties in static spacing override classes Aug 15, 2022
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2770 August 15, 2022 19:01 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2770 August 15, 2022 19:01 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2770 August 15, 2022 19:02 Inactive
@vanitabarrett vanitabarrett marked this pull request as ready for review August 15, 2022 19:11
Copy link
Contributor

@domoscargin domoscargin left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Do we have an issue written up to remove the class in the future?

@domoscargin domoscargin added this to the [NEXT] milestone Aug 15, 2022
@vanitabarrett
Copy link
Contributor Author

@36degrees does this match what you were expecting in terms of aliases and deprecating the old class?

@vanitabarrett
Copy link
Contributor Author

@domoscargin not yet, but I have got a checklist item to do that on the issue card

Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻 Just one minor comment about the deprecation comment.

.govuk-\!-margin-static-0,
.govuk-\!-static-margin-0 {
  margin: 0 !important; }

.govuk-\!-margin-top-static-0,
.govuk-\!-static-margin-top-0 {
  margin-top: 0 !important; }

.govuk-\!-margin-right-static-0,
.govuk-\!-static-margin-right-0 {
  margin-right: 0 !important; }

.govuk-\!-margin-bottom-static-0,
.govuk-\!-static-margin-bottom-0 {
  margin-bottom: 0 !important; }

.govuk-\!-margin-left-static-0,
.govuk-\!-static-margin-left-0 {
  margin-left: 0 !important; }

.govuk-\!-margin-static-1,
.govuk-\!-static-margin-1 {
  margin: 5px !important; }

.govuk-\!-margin-top-static-1,
.govuk-\!-static-margin-top-1 {
  margin-top: 5px !important; }

.govuk-\!-margin-right-static-1,
.govuk-\!-static-margin-right-1 {
  margin-right: 5px !important; }

.govuk-\!-margin-bottom-static-1,
.govuk-\!-static-margin-bottom-1 {
  margin-bottom: 5px !important; }

.govuk-\!-margin-left-static-1,
.govuk-\!-static-margin-left-1 {
  margin-left: 5px !important; }

.govuk-\!-margin-static-2,
.govuk-\!-static-margin-2 {
  margin: 10px !important; }

.govuk-\!-margin-top-static-2,
.govuk-\!-static-margin-top-2 {
  margin-top: 10px !important; }

.govuk-\!-margin-right-static-2,
.govuk-\!-static-margin-right-2 {
  margin-right: 10px !important; }

.govuk-\!-margin-bottom-static-2,
.govuk-\!-static-margin-bottom-2 {
  margin-bottom: 10px !important; }

.govuk-\!-margin-left-static-2,
.govuk-\!-static-margin-left-2 {
  margin-left: 10px !important; }

.govuk-\!-margin-static-3,
.govuk-\!-static-margin-3 {
  margin: 15px !important; }

.govuk-\!-margin-top-static-3,
.govuk-\!-static-margin-top-3 {
  margin-top: 15px !important; }

.govuk-\!-margin-right-static-3,
.govuk-\!-static-margin-right-3 {
  margin-right: 15px !important; }

.govuk-\!-margin-bottom-static-3,
.govuk-\!-static-margin-bottom-3 {
  margin-bottom: 15px !important; }

.govuk-\!-margin-left-static-3,
.govuk-\!-static-margin-left-3 {
  margin-left: 15px !important; }

.govuk-\!-margin-static-4,
.govuk-\!-static-margin-4 {
  margin: 20px !important; }

.govuk-\!-margin-top-static-4,
.govuk-\!-static-margin-top-4 {
  margin-top: 20px !important; }

.govuk-\!-margin-right-static-4,
.govuk-\!-static-margin-right-4 {
  margin-right: 20px !important; }

.govuk-\!-margin-bottom-static-4,
.govuk-\!-static-margin-bottom-4 {
  margin-bottom: 20px !important; }

.govuk-\!-margin-left-static-4,
.govuk-\!-static-margin-left-4 {
  margin-left: 20px !important; }

.govuk-\!-margin-static-5,
.govuk-\!-static-margin-5 {
  margin: 25px !important; }

.govuk-\!-margin-top-static-5,
.govuk-\!-static-margin-top-5 {
  margin-top: 25px !important; }

.govuk-\!-margin-right-static-5,
.govuk-\!-static-margin-right-5 {
  margin-right: 25px !important; }

.govuk-\!-margin-bottom-static-5,
.govuk-\!-static-margin-bottom-5 {
  margin-bottom: 25px !important; }

.govuk-\!-margin-left-static-5,
.govuk-\!-static-margin-left-5 {
  margin-left: 25px !important; }

.govuk-\!-margin-static-6,
.govuk-\!-static-margin-6 {
  margin: 30px !important; }

.govuk-\!-margin-top-static-6,
.govuk-\!-static-margin-top-6 {
  margin-top: 30px !important; }

.govuk-\!-margin-right-static-6,
.govuk-\!-static-margin-right-6 {
  margin-right: 30px !important; }

.govuk-\!-margin-bottom-static-6,
.govuk-\!-static-margin-bottom-6 {
  margin-bottom: 30px !important; }

.govuk-\!-margin-left-static-6,
.govuk-\!-static-margin-left-6 {
  margin-left: 30px !important; }

.govuk-\!-margin-static-7,
.govuk-\!-static-margin-7 {
  margin: 40px !important; }

.govuk-\!-margin-top-static-7,
.govuk-\!-static-margin-top-7 {
  margin-top: 40px !important; }

.govuk-\!-margin-right-static-7,
.govuk-\!-static-margin-right-7 {
  margin-right: 40px !important; }

.govuk-\!-margin-bottom-static-7,
.govuk-\!-static-margin-bottom-7 {
  margin-bottom: 40px !important; }

.govuk-\!-margin-left-static-7,
.govuk-\!-static-margin-left-7 {
  margin-left: 40px !important; }

.govuk-\!-margin-static-8,
.govuk-\!-static-margin-8 {
  margin: 50px !important; }

.govuk-\!-margin-top-static-8,
.govuk-\!-static-margin-top-8 {
  margin-top: 50px !important; }

.govuk-\!-margin-right-static-8,
.govuk-\!-static-margin-right-8 {
  margin-right: 50px !important; }

.govuk-\!-margin-bottom-static-8,
.govuk-\!-static-margin-bottom-8 {
  margin-bottom: 50px !important; }

.govuk-\!-margin-left-static-8,
.govuk-\!-static-margin-left-8 {
  margin-left: 50px !important; }

.govuk-\!-margin-static-9,
.govuk-\!-static-margin-9 {
  margin: 60px !important; }

.govuk-\!-margin-top-static-9,
.govuk-\!-static-margin-top-9 {
  margin-top: 60px !important; }

.govuk-\!-margin-right-static-9,
.govuk-\!-static-margin-right-9 {
  margin-right: 60px !important; }

.govuk-\!-margin-bottom-static-9,
.govuk-\!-static-margin-bottom-9 {
  margin-bottom: 60px !important; }

.govuk-\!-margin-left-static-9,
.govuk-\!-static-margin-left-9 {
  margin-left: 60px !important; }

.govuk-\!-padding-static-0,
.govuk-\!-static-padding-0 {
  padding: 0 !important; }

.govuk-\!-padding-top-static-0,
.govuk-\!-static-padding-top-0 {
  padding-top: 0 !important; }

.govuk-\!-padding-right-static-0,
.govuk-\!-static-padding-right-0 {
  padding-right: 0 !important; }

.govuk-\!-padding-bottom-static-0,
.govuk-\!-static-padding-bottom-0 {
  padding-bottom: 0 !important; }

.govuk-\!-padding-left-static-0,
.govuk-\!-static-padding-left-0 {
  padding-left: 0 !important; }

.govuk-\!-padding-static-1,
.govuk-\!-static-padding-1 {
  padding: 5px !important; }

.govuk-\!-padding-top-static-1,
.govuk-\!-static-padding-top-1 {
  padding-top: 5px !important; }

.govuk-\!-padding-right-static-1,
.govuk-\!-static-padding-right-1 {
  padding-right: 5px !important; }

.govuk-\!-padding-bottom-static-1,
.govuk-\!-static-padding-bottom-1 {
  padding-bottom: 5px !important; }

.govuk-\!-padding-left-static-1,
.govuk-\!-static-padding-left-1 {
  padding-left: 5px !important; }

.govuk-\!-padding-static-2,
.govuk-\!-static-padding-2 {
  padding: 10px !important; }

.govuk-\!-padding-top-static-2,
.govuk-\!-static-padding-top-2 {
  padding-top: 10px !important; }

.govuk-\!-padding-right-static-2,
.govuk-\!-static-padding-right-2 {
  padding-right: 10px !important; }

.govuk-\!-padding-bottom-static-2,
.govuk-\!-static-padding-bottom-2 {
  padding-bottom: 10px !important; }

.govuk-\!-padding-left-static-2,
.govuk-\!-static-padding-left-2 {
  padding-left: 10px !important; }

.govuk-\!-padding-static-3,
.govuk-\!-static-padding-3 {
  padding: 15px !important; }

.govuk-\!-padding-top-static-3,
.govuk-\!-static-padding-top-3 {
  padding-top: 15px !important; }

.govuk-\!-padding-right-static-3,
.govuk-\!-static-padding-right-3 {
  padding-right: 15px !important; }

.govuk-\!-padding-bottom-static-3,
.govuk-\!-static-padding-bottom-3 {
  padding-bottom: 15px !important; }

.govuk-\!-padding-left-static-3,
.govuk-\!-static-padding-left-3 {
  padding-left: 15px !important; }

.govuk-\!-padding-static-4,
.govuk-\!-static-padding-4 {
  padding: 20px !important; }

.govuk-\!-padding-top-static-4,
.govuk-\!-static-padding-top-4 {
  padding-top: 20px !important; }

.govuk-\!-padding-right-static-4,
.govuk-\!-static-padding-right-4 {
  padding-right: 20px !important; }

.govuk-\!-padding-bottom-static-4,
.govuk-\!-static-padding-bottom-4 {
  padding-bottom: 20px !important; }

.govuk-\!-padding-left-static-4,
.govuk-\!-static-padding-left-4 {
  padding-left: 20px !important; }

.govuk-\!-padding-static-5,
.govuk-\!-static-padding-5 {
  padding: 25px !important; }

.govuk-\!-padding-top-static-5,
.govuk-\!-static-padding-top-5 {
  padding-top: 25px !important; }

.govuk-\!-padding-right-static-5,
.govuk-\!-static-padding-right-5 {
  padding-right: 25px !important; }

.govuk-\!-padding-bottom-static-5,
.govuk-\!-static-padding-bottom-5 {
  padding-bottom: 25px !important; }

.govuk-\!-padding-left-static-5,
.govuk-\!-static-padding-left-5 {
  padding-left: 25px !important; }

.govuk-\!-padding-static-6,
.govuk-\!-static-padding-6 {
  padding: 30px !important; }

.govuk-\!-padding-top-static-6,
.govuk-\!-static-padding-top-6 {
  padding-top: 30px !important; }

.govuk-\!-padding-right-static-6,
.govuk-\!-static-padding-right-6 {
  padding-right: 30px !important; }

.govuk-\!-padding-bottom-static-6,
.govuk-\!-static-padding-bottom-6 {
  padding-bottom: 30px !important; }

.govuk-\!-padding-left-static-6,
.govuk-\!-static-padding-left-6 {
  padding-left: 30px !important; }

.govuk-\!-padding-static-7,
.govuk-\!-static-padding-7 {
  padding: 40px !important; }

.govuk-\!-padding-top-static-7,
.govuk-\!-static-padding-top-7 {
  padding-top: 40px !important; }

.govuk-\!-padding-right-static-7,
.govuk-\!-static-padding-right-7 {
  padding-right: 40px !important; }

.govuk-\!-padding-bottom-static-7,
.govuk-\!-static-padding-bottom-7 {
  padding-bottom: 40px !important; }

.govuk-\!-padding-left-static-7,
.govuk-\!-static-padding-left-7 {
  padding-left: 40px !important; }

.govuk-\!-padding-static-8,
.govuk-\!-static-padding-8 {
  padding: 50px !important; }

.govuk-\!-padding-top-static-8,
.govuk-\!-static-padding-top-8 {
  padding-top: 50px !important; }

.govuk-\!-padding-right-static-8,
.govuk-\!-static-padding-right-8 {
  padding-right: 50px !important; }

.govuk-\!-padding-bottom-static-8,
.govuk-\!-static-padding-bottom-8 {
  padding-bottom: 50px !important; }

.govuk-\!-padding-left-static-8,
.govuk-\!-static-padding-left-8 {
  padding-left: 50px !important; }

.govuk-\!-padding-static-9,
.govuk-\!-static-padding-9 {
  padding: 60px !important; }

.govuk-\!-padding-top-static-9,
.govuk-\!-static-padding-top-9 {
  padding-top: 60px !important; }

.govuk-\!-padding-right-static-9,
.govuk-\!-static-padding-right-9 {
  padding-right: 60px !important; }

.govuk-\!-padding-bottom-static-9,
.govuk-\!-static-padding-bottom-9 {
  padding-bottom: 60px !important; }

.govuk-\!-padding-left-static-9,
.govuk-\!-static-padding-left-9 {
  padding-left: 60px !important; }

#{$property}: govuk-spacing($spacing-point) !important;
}

@each $direction in $_spacing-directions {

.govuk-\!-#{$property}-#{$direction}-static-#{$spacing-point} {
/// govuk-#{$property}-#{$direction}-static-#{$spacing-point} is deprecated. Use .govuk-\!-static-#{$property}-#{$direction}-#{$spacing-point} instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

Given this applies to the non-directional classes as well, do we either want to add a second comment there or move this either to the top or into the Sassdoc for the function and make it cover both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I've given it a go, but found it difficult to write anything generic, so it's a bit long... Welcome any comments!

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks great to me 👍🏻

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2770 August 16, 2022 11:10 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2770 August 16, 2022 13:17 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2770 August 17, 2022 08:52 Inactive
CHANGELOG.md Outdated Show resolved Hide resolved
We incorrectly shipped static spacing override classes in the format
`govuk-!-padding-static` and `govuk-!-margin-static`, when we intended
them to be in the format `govuk-!-static`.

Our documentation refers to `govuk-!static`. We should update the classes to match,
but keep the existing classes for now as people may already be using them.
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2770 August 17, 2022 09:36 Inactive
@vanitabarrett vanitabarrett merged commit 3ac74ea into main Aug 17, 2022
@vanitabarrett vanitabarrett deleted the correct-static-spacing-class branch August 17, 2022 09:46
@36degrees 36degrees mentioned this pull request Aug 18, 2022
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.

Static spacing override classes don't start with govuk-!-static (as documented)
5 participants