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

SSR: Mark masterbar and layout files as @ssr-ready #2271

Merged
merged 1 commit into from
Jan 15, 2016

Conversation

ehg
Copy link
Member

@ehg ehg commented Jan 11, 2016

Part of #1551

Mark everything necessary to render the MasterBar on logged-out /design as @ssr-ready. Gridicons manually updated for now.

TODO:

  • Look at the rest of the components in shared/, see what we can mark as ready
  • Figure out what to do for config

To test:

  • Restart make run
  • Make sure pragma checker isn't complaining

@ehg ehg self-assigned this Jan 11, 2016
@ehg ehg added this to the Themes: Showcase M3-LoggedOut milestone Jan 11, 2016
@ehg ehg force-pushed the update/mark-masterbar-ssr-ready branch from e47d61b to e14d790 Compare January 12, 2016 12:50
@ehg ehg added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Jan 12, 2016
@@ -1,3 +1,4 @@
/** @ssr-ready **/

/* !!!
IF YOU ARE EDITING gridicon/index.jsx
Copy link
Member

Choose a reason for hiding this comment

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

Potentially OT, but is this warning still current?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, there's a PR in the Gridicons repo for this. 136-gh-gridicons

Copy link
Member

Choose a reason for hiding this comment

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

Noice.

@seear
Copy link
Contributor

seear commented Jan 14, 2016

👍

1 similar comment
@mcsf
Copy link
Member

mcsf commented Jan 14, 2016

👍

@ehg
Copy link
Member Author

ehg commented Jan 14, 2016

So, I think if I merge this, it'll immediately cause everyone's make run process to abort, since config is only regenerated on startup. Gonna push a fix :)

@ehg ehg force-pushed the update/mark-masterbar-ssr-ready branch from e14d790 to e3c6314 Compare January 14, 2016 15:42
@ehg
Copy link
Member Author

ehg commented Jan 14, 2016

OK, I'm ignoring config in e3c6314. For testing this, pull the changes, then switch branch to master, it shouldn't exit.

- Gridicons manually updated, but should merge fine.
- Ignore config — for these situations where a module has different code for
client/server, we shouldn't mark them as ssr-ready.
@ehg ehg force-pushed the update/mark-masterbar-ssr-ready branch from 9422707 to f7e7448 Compare January 15, 2016 12:04
@mcsf
Copy link
Member

mcsf commented Jan 15, 2016

Can confirm.

👍

ehg added a commit that referenced this pull request Jan 15, 2016
SSR: Mark masterbar and layout files as `@ssr-ready`
@ehg ehg merged commit 3721bd8 into master Jan 15, 2016
@ehg ehg deleted the update/mark-masterbar-ssr-ready branch January 15, 2016 12:52
@ehg ehg removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants