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 sass warnings #3323

Merged
merged 1 commit into from
Sep 5, 2024
Merged

Fix sass warnings #3323

merged 1 commit into from
Sep 5, 2024

Conversation

andysellick
Copy link
Contributor

@andysellick andysellick commented Sep 5, 2024

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

What

Fixes some Sass compilation warnings, that look like this:

13:28:45 css.1  |    ┌──> app/assets/stylesheets/helpers/_publisher-metadata-with-logo.scss
13:28:45 css.1  | 7  │     margin-left: govuk-spacing(3);
13:28:45 css.1  |    │     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ declaration
13:28:45 css.1  |    ╵
13:28:45 css.1  |    ┌──> ../../../.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/govuk_publishing_components-43.0.2/node_modules/govuk-frontend/dist/govuk/helpers/_clearfix.scss
13:28:45 css.1  | 10 │ ┌   &::after {
13:28:45 css.1  | 11 │ │     content: "";
13:28:45 css.1  | 12 │ │     display: block;
13:28:45 css.1  | 13 │ │     clear: both;
13:28:45 css.1  | 14 │ │   }
13:28:45 css.1  |    │ └─── nested rule
13:28:45 css.1  |    ╵
13:28:45 css.1  |     app/assets/stylesheets/helpers/_publisher-metadata-with-logo.scss 7:3  @import
13:28:45 css.1  |     app/assets/stylesheets/application.scss 19:9                           root stylesheet

This involves moving all the @ declarations to after the other statements. I think all the changes here are fine, the only one I hesitated over was moving govuk-font after a text colour setting, when it was previously before - I don't think govuk-font sets a colour, but I can't find the right line in govuk-frontend to confirm that.

Why

There was a change to a dependency recently where this arrangement of selectors was suddenly required, so we're having to update quite a few Sass files to silence these warnings.

Visual changes

None.

@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-3323 September 5, 2024 13:17 Inactive
@andysellick andysellick requested a review from AshGDS September 5, 2024 13:35
@andysellick andysellick marked this pull request as ready for review September 5, 2024 13:35
Copy link
Contributor

@AshGDS AshGDS left a comment

Choose a reason for hiding this comment

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

@andysellick andysellick merged commit 7253b15 into main Sep 5, 2024
12 checks passed
@andysellick andysellick deleted the fix-sass-warning branch September 5, 2024 14:05
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.

3 participants