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

add global stylesheet example #1016

Merged
merged 4 commits into from
Feb 11, 2017
Merged

add global stylesheet example #1016

merged 4 commits into from
Feb 11, 2017

Conversation

davibe
Copy link
Contributor

@davibe davibe commented Feb 6, 2017

I know "global" styles are not a priority for 2.0 release but i think this example really solves it for many users in the mean time.

Fixes #544

@niksurff
Copy link

niksurff commented Feb 6, 2017

Hey @davibe , thanks for the example.

Just today I was looking to add an external CSS file. I got it working with babel-plugin-inline-import.

After a quick look into your example I noticed a couple things:

  1. One should inject a string of CSS into a jsx <style>-tag with dangerouslySetInnerHTML otherwise the string will get escaped (noticeable when using " or ' as in font-family: "Comic Sans", serif).
  2. My understanding is that on the server side code doesn't get bundled with Webpack so your SCSS example might not be working when rendering server side?

@davibe
Copy link
Contributor Author

davibe commented Feb 7, 2017

  1. Right, i fixed this.
  2. It works fine, i just checked. You can try with curl and see the initial page content. Last time i read the next.js sources I understood that webpack is used to generate the files for the client but those are the same files that are loaded, reloaded and executed by the server when it has to render. So webpack babel transpilation has already occurred for both sides, unless i missed something :)

@rauchg
Copy link
Member

rauchg commented Feb 7, 2017

You're all doing such awesome work 💖

@@ -0,0 1,2 @@
.next
Copy link
Member

Choose a reason for hiding this comment

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

No other examples include a .gitignore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it does not hurt but i'll remove it

@@ -0,0 1,31 @@
README
Copy link
Member

Choose a reason for hiding this comment

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

Can you port this documentation to the same style as https://github.com/zeit/next.js/tree/master/examples/basic-css. All examples have kind of the same readme ❤️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@timneutkens timneutkens left a comment

Choose a reason for hiding this comment

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

Besides the 2 points I had this looks great, it's a port of your github repo right 👍

@davibe
Copy link
Contributor Author

davibe commented Feb 7, 2017

Yes. I would like to keep it in sync with the repo because i can track issues there and sync-back from time to time

Copy link
Member

@timneutkens timneutkens left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@@ -0,0 1,687 @@
<img width="112" alt="screen shot 2016-10-25 at 2 37 27 pm" src="https://cloud.githubusercontent.com/assets/13041/19686250/971bf7f8-9ac0-11e6-975c-188defd82df1.png">
Copy link
Member

Choose a reason for hiding this comment

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

Hey @davibe seems like you included the main repo readme 😅 I meant include this one but updated to your example 👍

https://raw.githubusercontent.com/zeit/next.js/master/examples/basic-css/README.md

@timneutkens timneutkens merged commit 19f1125 into vercel:master Feb 11, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants