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

[docs] Improve the Server Rendering section #6070

Merged
merged 1 commit into from
Feb 7, 2017
Merged

[docs] Improve the Server Rendering section #6070

merged 1 commit into from
Feb 7, 2017

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Feb 6, 2017

  • PR has tests / docs demo, and is linted.
  • Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
  • Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

It's highly inspired by http://redux.js.org/docs/recipes/ServerRendering.html.

Closes #6065. cc @kybarg

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation next labels Feb 6, 2017
Copy link
Member

@mbrookes mbrookes left a comment

Choose a reason for hiding this comment

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

next docs are going to be amazing!

On the client side, the default value is `navigator.userAgent`.
But on the server side, the `navigator` is `undefined`. You need to provide it to Material-UI.
Material-UI was designed from the ground-up with the constraint of rendering on the Server.
But it's up to users to makes sure it's correctly integrated.
Copy link
Member

Choose a reason for hiding this comment

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

'on the Server, but'

But it's up to users to makes sure it's correctly integrated.
We must provide to the page the needed style.
It's important because you don't want the interface to flicker.
Otherwise, the page will render with just the HTML, then wait for the CSS to be injected.
Copy link
Member

Choose a reason for hiding this comment

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

Replace 11 - 13 with:

"It's important that we provide the page with the required CSS, otherwise the page will render with just the HTML then wait for the CSS to be injected by the client, causing it to flicker."

- create a fresh, new `styleManager` and `theme` instance on every request;
- render the React tree with the server-side API and the instance;
- pull the CSS out of the `styleManager`;
- and then pass the CSS along to the client.
Copy link
Member

Choose a reason for hiding this comment

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

Up to you, but I would make this a numbered list, and begin each item with a capital, and end with each a full-stop. The content itself is good.

For instance, you can provide it like this:
## Setting Up

In the follow recipe, we are going to look at how to set up server-side rendering.
Copy link
Member

Choose a reason for hiding this comment

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

following

@mbrookes mbrookes added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI and removed PR: Needs Review labels Feb 7, 2017
@oliviertassinari oliviertassinari dismissed mbrookes’s stale review February 7, 2017 19:11

Thanks for the review! I have fixed everything. Let's merge.

@oliviertassinari oliviertassinari removed PR: Needs Review PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels Feb 7, 2017
@oliviertassinari oliviertassinari merged commit 2226613 into mui:next Feb 7, 2017
@oliviertassinari oliviertassinari deleted the next-docs-server-rendering branch February 7, 2017 19:12
@chadfurman
Copy link

@oliviertassinari @mbrookes This documentation was lost along the way -- is this still the correct way to do server rendering in @next?

@chadfurman
Copy link

@oliviertassinari
Copy link
Member Author

You can also find this integration with next.js https://github.com/zeit/next.js/tree/v3-beta/examples/with-material-ui-next

@chadfurman
Copy link

Following the link in this repo, I was able to get material-ui to render with animations! Thank you :D

Now I can see I need to create a color spread for my custom accent color ;)

Nice job on the docs, helped a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[style] Server side rendering
4 participants