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

Themes: Merge layout/logged-out-design back into layout/logged-out #3082

Closed
ockham opened this issue Feb 4, 2016 · 2 comments
Closed

Themes: Merge layout/logged-out-design back into layout/logged-out #3082

ockham opened this issue Feb 4, 2016 · 2 comments

Comments

@ockham
Copy link
Contributor

ockham commented Feb 4, 2016

Ideally, there'd be only one logged-out layout (for consistency and ease of maintenance) which we could also use for server-side rendering. The main differences between the two logged-out layouts currently are:

  • layout/logged-out-design uses ThemesHead
    • I'm wondering a bit if Helmet is really that beneficial. We have two levels of wrapping there (layout/head and my-sites/themes/head), and can't really leverage React anyway but need to toss around props and then extract those data again using rewind(). Wouldn't it be easier to just put that stuff into a Redux subtree? (And dispatch inside our controllers.) @mcsf?
  • layout/logged-out uses the old notices store
@ockham ockham added this to the Themes: Future milestone Feb 4, 2016
@ockham ockham changed the title Themes: Merge back layout/logged-out-design into layout/logged-out Themes: Merge layout/logged-out-design back into layout/logged-out Feb 4, 2016
@mcsf
Copy link
Member

mcsf commented Feb 8, 2016

I'm wondering a bit if Helmet is really that beneficial. We have two levels of wrapping there (layout/head and my-sites/themes/head), and can't really leverage React anyway but need to toss around props and then extract those data again using rewind(). Wouldn't it be easier to just put that stuff into a Redux subtree? (And dispatch inside our controllers.) @mcsf?

As we discussed in a video walkthrough, sharing for posterity: Helmet would make a lot less sense if Calypso were already fully Redux'd and all state (sections, layout, routing?, etc.) lived in clear places of the global tree isomorphically. I think moving from Helmet to Redux will be a relatively straightforward task once we feel ready for it, so in the meantime we can benefit from the momentum advantage we're getting from Helmet.

@ockham
Copy link
Contributor Author

ockham commented Feb 9, 2016

Closing this issue since layout/logged-out doesn't exist anymore (see #2964). Maybe we should rename layout/logged-out-design to layout/logged-out tho.

@ockham ockham closed this as completed Feb 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants