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

Remove compass boilerplate #177

Merged
merged 3 commits into from
Feb 1, 2017
Merged

Remove compass boilerplate #177

merged 3 commits into from
Feb 1, 2017

Conversation

sir-dunxalot
Copy link
Owner

This PR removes the dependency on the compass-boilerplate library, which was causing this addon's test suite to fail. Whilst the compass-boilerplate test suite was able to be fixed to a passing state, various environments upgrading their node versions have appear to bring incompatibilities causing consuming app's test suites to fail. Specifically, the following error appeared in Ember CLI and Travis builds of this addon:

Error: File to import not found or unreadable: node_modules/compass-mixins/lib/compass.

By removing compass-boilerplate from this project, the test suite is restored to a passing build.

The outputted CSS of the dummy app, which was what we used compass-boilerplate for, is styled the same. We're just writing the scss in a more explicit way.

@import 'compass-boilerplate-vendor/html5bp';
@import 'compass-boilerplate';
@import 'vendor/normalize';
@import 'vendor/html5bp';
Copy link
Owner Author

Choose a reason for hiding this comment

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

As a quick fix, I have moved the normalize and html5bp files into this project. These files are added in this PR below.

@include flex-direction(column);
justify-content: center;
align-items: center;
flex-direction: column;
Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm making the assumption that ember developer are using up-to-date browsers, which don't need vendor prefixes. If we want to make our dummy app more cross-browser friendly, we could use ember-cli-postcss. I think that's outside the scope of this PR.

Copy link
Owner Author

Choose a reason for hiding this comment

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

#178 adds postcss and autoprefixer to the dummy app to accommodate for these changes.

@@ -1,6 +1,5 @@
@import 'compass-boilerplate-vendor/normalize';
Copy link
Owner Author

Choose a reason for hiding this comment

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

Note: all the changes in this file only affect the dummy app.

@@ -0,0 +1,245 @@
/*! HTML5 Boilerplate v5.2.0 | MIT License | https://html5boilerplate.com/ */
Copy link
Owner Author

@sir-dunxalot sir-dunxalot Feb 1, 2017

Choose a reason for hiding this comment

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

File copied from compass-boilerplate.

@@ -0,0 +1,424 @@
/*! normalize.css v3.0.3 | MIT License | github.com/necolas/normalize.css */
Copy link
Owner Author

Choose a reason for hiding this comment

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

File copied from compass-boilerplate.

@sir-dunxalot
Copy link
Owner Author

When this PR is merged, we need to redeploy the dummy app:

git checkout master # make sure you're on master branch
ember github-pages:commit --message "Some commit message" # Builds the app
git push origin gh-pages:gh-pages # Deploys the app

This was referenced Feb 1, 2017
Copy link
Contributor

@a15n a15n left a comment

Choose a reason for hiding this comment

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

LGTM

@a15n a15n merged commit a7bbd62 into master Feb 1, 2017
@sir-dunxalot sir-dunxalot deleted the remove-compass-boilerplate branch September 21, 2018 21:00
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.

2 participants