Remove govuk elements sass from the app folder #208
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gemmaleigh
force-pushed
the
remove-govuk-elements-sass-from-the-app-folder
branch
from
70b7316 to
2342ff5
Compare
|
+1 from me. Seems more intuitive not to be copying files from a module into the |
joelanman
pushed a commit
that referenced
this pull request
May 31, 2016
# 0.17.3 - Fix colour of H2 headings in footer # 0.17.2 - Fix a bug with the skip-to-content link and iOS Voiceover - Migrate @Viewport statement from govuk_frontend_toolkit # 0.17.1 - Reduce file size of template: removes HTML comments, `type` attributes on scripts, and uses HTML5 charset declaration. #208 - Switch external link media query to be mobile first #205 - Sass file cleanups - Replace old grid mixins with newer grid from frontend toolkit #134 - Remove duplicate grey variables #201
feedmypixel
pushed a commit
to hmrc/govuk_prototype_kit
that referenced
this pull request
May 31, 2016
# 0.17.3 - Fix colour of H2 headings in footer # 0.17.2 - Fix a bug with the skip-to-content link and iOS Voiceover - Migrate @Viewport statement from govuk_frontend_toolkit # 0.17.1 - Reduce file size of template: removes HTML comments, `type` attributes on scripts, and uses HTML5 charset declaration. alphagov#208 - Switch external link media query to be mobile first alphagov#205 - Sass file cleanups - Replace old grid mixins with newer grid from frontend toolkit alphagov#134 - Remove duplicate grey variables alphagov#201
|
looks good, means it lines up with the way we're doing it elsewhere. Also fixes the weird situation where these files were bring included in the kit (in |
gemmaleigh
changed the title
|
@joelanman, can you merge this one please? |
Merged
gemmaleigh
force-pushed
the
remove-govuk-elements-sass-from-the-app-folder
branch
from
2342ff5 to
8e5e5e9
Compare
These already exist in govuk-modules/govuk-elements-sass, so there is no need to also copy them to /app/
There is no need for these to be copied to app/assets/sass, as they will be overridden if edited. Remove them to avoid confusion.
These files won’t need to be copied from govuk/modules. In theory govuk_modules isn’t needed either and these paths could be updated to /node_modules/govuk_repo_name but it provides a way to see what lives in each repo.
As it might not be clear where _govuk-elements.scss is imported from, update the comment.
|
👍 I came a cropper with this issue just the other day. |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds
/govuk-modules/govuk-elements-sasstoincludePaths, so that the GOV.UK elements Sass files can remain in/govuk-modules/govuk-elements-sass/without the need for an additional copy task.As these files are overridden each time the app is run, there are probably a number of users who have tried to edit the files only to find that their changes have been lost.
This way, the
govuk-elements-sassfiles can be viewed in the same way as the template and toolkit files -by looking in/govuk-modules/(as there is still an extra copy step to govuk-modules) but they won't appear in/app/assets/sass/.