Automattic / wp-calypso

The JavaScript and API powered WordPress.com
https://developer.wordpress.com
GNU General Public License v2.0
12.42k stars 1.99k forks source link

Themes: Move Components around #9671

Closed ockham closed 7 years ago

ockham commented 7 years ago
  1. Move ThemesSearchCard from ThemesSelection to ThemeShowcase
  2. ThemeShowcase shouldn't add ThemeSelection as a child automatically anymore. Instead, logged-out.jsx, multi-site.jsx, single-site-wpcom.jsx, and single-site-jetpack.jsx should do so (as they're already doing with other components).
  3. Instead of using connectOptions on the top-level components in those four files, we should then just wrap the respectively imported ThemeSelection components there (or, in the case of multi-site.jsx, wrap ThemeSelection in ThemeSiteSelectorModal first).

This will

folletto commented 7 years ago

Brief analysis:

screen shot 2016-11-29 at 16 53 00

ockham commented 7 years ago

I've thought about this and tinkered with it some more.

The issues we were running into with #9792 and #9823 weren't just due to implementation details, but due to a more fundamental complexity: ThemesSelection isn't the only options consumer inside ThemeShowcase -- ThemePreview is, too. With the current component arch, we might have to duplicate the preview. The cleaner solution would be something like #9832, plus moving some logic to determine the default and secondary option inside theme-options, but hopefully we'll just get away with duplicating the preview.

First and foremost however, we cannot even experiment with duplicating ThemesSelection for our Jetpack view before #8785, since due to the old Redux arch, queries will collide, and bad things will happen, rendering the Jetpack view unusable (I've tried this on a local branch).

Bottom line: I'll try to finish #8785 ASAP. We should revisit this only afterwards. I'll link an experimental branch.

ockham commented 7 years ago

Closing for now. Will revisit re-organizing our currently anarchic component hierarchy after the JetPress rally.