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

Added solution to compile problem for npm users #10848

Merged
merged 2 commits into from
Jan 17, 2018

Conversation

jnystromdesign
Copy link
Contributor

@jnystromdesign jnystromdesign commented Jan 10, 2018

No description provided.

@@ -77,7 +77,8 @@ Finally, add an `@import` statement to the top of your primary Sass file. Refer
@import 'foundation';
```

You're also going to want a settings file for your project, which will allow you to modify the default styles of Foundation. **[Download the latest settings file here](https://raw.githubusercontent.com/zurb/foundation-sites/master/scss/settings/_settings.scss)**, add it to your project as `_settings.scss`, then import it *before* Foundation itself.
You're also going to want a settings file for your project, which will allow you to modify the default styles of Foundation. **[Download the latest settings file here](https://raw.githubusercontent.com/zurb/foundation-sites/master/scss/settings/_settings.scss)**, add it to your project as `_settings.scss`, then import it *before* Foundation itself.
For users of npm, you need to change `@import util/util` to a full path to the util (node_modules/foundation-sites/scss/util/util) in the settings file in order to compile it.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this concerns only to NPM users. All users with a package manager or a custom installation of Foundation may have the Foundation missing from the Sass includePaths and should edit utils/utils.

Maybe we could reformulate this to:

<div class="callout>
    The settings file needs to import `util/util` from Foundation. Please ensure that the Foundation folder is included in Sass or change `@import util/util` for it to points to the full path of the file. For example, NPM users may need to change the import to `node_modules/foundation-sites/scss/util/util`.
</div>

Copy link
Contributor

Choose a reason for hiding this comment

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

We could also add a message inside settings.scss

Copy link
Contributor

Choose a reason for hiding this comment

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

Or in webpack where we can use ~packagename

@ncoden
Copy link
Contributor

ncoden commented Jan 11, 2018

@jnystromdesign Would you have some time to discuss/provide the requested changes ? Or should be take care of this ?

@jnystromdesign
Copy link
Contributor Author

@ncoden Uhm. Etither callouts aren't showing in the markdown preview at GitHub or I've done something stupid. Sry. Just want to contribute, but I'm somewhat of a newb when it comes to collaboration on GitHub.

@jnystromdesign
Copy link
Contributor Author

@ncoden Any way to communicate that the util/util needs to be included, is fine for me (Am a big fan of heuristics). However I suggest it should be explained in the documentation and not in the codebase.

@ncoden
Copy link
Contributor

ncoden commented Jan 11, 2018

I think it should be explained in the documentation and in the codebase. For the callout, it's because the GitHub markdown preview doesn't have our "callout" class. This doc is not pure markdown and will be visible only in https://foundation.zurb.com/sites/docs.

Could you add a comment in the settings file, just above the import ?

@ncoden
Copy link
Contributor

ncoden commented Jan 17, 2018

I just remembered that foundation.scss is automatically generated by octophant. So I'm merging this. 👍 Thank you @jnystromdesign

@ncoden ncoden merged commit 10bd826 into foundation:master Jan 17, 2018
ncoden pushed a commit to ncoden/foundation-sites that referenced this pull request Jun 16, 2018
92d4bea Added solution to compile problem for npm users
b8779d6 Wrapped instructions in a callout

Signed-off-by: Nicolas Coden <nicolas@ncoden.fr>
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