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

Looks like sprockets v4.0.0 upgrade changed to use manifest file heav… #1406

Merged
merged 4 commits into from
Jan 9, 2020

Conversation

murny
Copy link
Contributor

@murny murny commented Jan 7, 2020

…ily, which was not configured correctly

Should fix travisCI Builds.

More info here:
https://github.com/rails/sprockets/blob/master/UPGRADING.md#manifestjs
and issue with our problem with solution here:
rails/sprockets#597

.community-collection {
.card {
// TODO: have real designer pick this
background-color: $cc-background;
background-color: $gray-100;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clean this up a bit. Don't need the secondary variable and TODO isn't really relevant anymore 😅

@@ -11,4 +11,4 @@
# Precompile additional assets.
# application.js, application.css, and all non-JS/CSS in the app/assets
# folder are already added.
Rails.application.config.assets.precompile += %w[admin.js]
# Rails.application.config.assets.precompile += %w[admin.js]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently you don't do this anymore in Sprockets v4.0.0, they want you to use manifest instead.

@@ -1,3 +1,5 @@
//= link_tree ../images
//= link_directory ../javascripts .js
//= link_directory ../stylesheets .css
Copy link
Contributor Author

@murny murny Jan 7, 2020

Choose a reason for hiding this comment

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

So this is the problem. It was loading it in a order we didn't want. We want to load just the application.css file and the order that is specified by that file.

rails/sprockets#597 (comment)

//= link_directory ../stylesheets .css

//= link application.css
//= link admin.js
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of Rails.application.config.assets.precompile += %w[admin.js] we put it here

Tested both quickly by running Rails s and looking at admin section. All seems good

@ualbertalib-bot
Copy link

ualbertalib-bot commented Jan 7, 2020

1 Warning
⚠️ There are code changes, but no corresponding tests. Please include tests if this PR introduces any modifications in behavior.

Generated by 🚫 Danger

@murny murny merged commit a213855 into integration_postmigration Jan 9, 2020
@murny murny deleted the Fix-Sass-undefined-var-issue branch January 9, 2020 00:27
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.

4 participants