Signup: Replace usage of ThemeThumbnail for ThemesList component on Theme Selection Signup Step #1079

oskosk · 2015-12-01T00:03:39Z

ThemesList is a reusable component appropriate for the function that ThemeSelection Step is providing.

ThemeThumbnail will no longer be necessary. The theme selection step still handles a headStart flow providing {theme: 'pub/twentyfifteen'} as a default dependency.

See #256 and #674 .

cc @scruffian

Testing instructions

Regular theme selection

Load http://calypso.localhost:3000/start (you'll be redirected to /start/themes)
Select a theme.
Follow domain registration (and user signup if testing anonymously).
After completing registration, go to switch sites, find your new site.
Go to themes and check the theme you selected is the one active for your new site.

Headstart flow

Load http://calypso.localhost:3000/start/headstart (you'll be redirected to /start/themes)
Don't select any theme and click the Skip button. (This way, theme twentyfifteen will be selected by default.
Follow domain registration (and user signup if testing anonymously).
After completing registration, go to switch sites, find your new site.
Go to themes and check the active theme for your new site is Twenty Fifteen.

/design theme selection for existing sites

Once logged in, go to switch sites, find one of your sites.
On sidebar, click Themes under Personalize.
The new theme gallery is using ThemesList and thus, the updated Theme component. There shouldn't be any differences with its current behaviour and appearance as the default Theme behaviour is used here.

scruffian · 2015-12-01T11:12:14Z

client/signup/steps/theme-selection/style.scss

-	padding: 12px 0;
-	text-align: center;
+.themes-list .theme__active-focus {
+	opacity: 0.0;


Two things. Firstly we shouldn't use opacity to hide stuff, secondly, we don't need the .0.

Ok, will proceed to update the Theme component to show the preview depending on a boolean .prop

Third, any CSS change needs to be namespaced properly for the context it's being used on: this change is being applied everywhere! :)

Mhh, sorry for that! This css rule won't stay there as I'll update the Theme component to do what this line was supposed to do. Thanks

scruffian · 2015-12-01T11:18:37Z

cc @folletto - this changes the theme thumbnails from this:

to:

Does that look ok to you?

scruffian · 2015-12-01T11:21:56Z

client/signup/steps/theme-selection/index.jsx

@@ -30,28 +34,55 @@ module.exports = React.createClass( {
 		};
 	},

-	renderThemes: function() {
+	handleScreenshotClick: function( theme ) {


I assume this code is from the old theme thumbnail component? Can we remove that at the same time?

I don't fully get what you meant. handlScreenshotClick is passed as prop to ThemesList and forwarded to Theme in order to instruct what to do when a screenshot is clicked. In this case this method submits the step.

scruffian · 2015-12-01T11:23:15Z

Looking good!

folletto · 2015-12-01T11:53:31Z

Heya!

Yes, style wise it's correct, however there are two issues:

The overlay has disappeared (see the opacity: 0.0 note above). No CSS should be added at the general namespacing level if it's to be used only in a specific section (in this case, the funnel).
The overlay should thus stay there, with just a change of label. As a first proposal for the label, I'd go for: "Pick".

oskosk · 2015-12-01T11:59:35Z

Awesome @folletto, thanks! @scruffian I'll make the Theme component overlay text depend on a prop instead of hiding/showing it depending on a boolean prop. Defaulting to 'preview'.

oskosk · 2015-12-01T13:13:58Z

Updated Theme component to let you choose the actionLabel text
Updated theme-selection to use 'Pick' text for the label.

cc @scruffian @folletto

Before this pull request:

After

folletto · 2015-12-01T13:23:11Z

Design wise 👍

scruffian · 2015-12-01T18:19:54Z

LGTM 👍

scruffian · 2015-12-01T18:20:20Z

As a followup it would be good to use the same component in the DSS flow...

ThemesList is a reusable component appropriate for the function that ThemeSelection Step is providing. ThemeThumbnail will no longer be necessary. The theme selection step still handles a headStart flow providing {theme: 'pub/twentyfifteen'} as a default dependency.

…s.actionLabel if theme is not active. The default label text if .props.active is falsey is the translated 'Preview' string. For a Theme instance having .props.active with a truthy value, the label text is the translated string 'Customize'.

The scss file for theme-selection is no longer needed and is also removed by this commit.

…e-list-component Signup: Replace usage of ThemeThumbnail for ThemesList component on Theme Selection Signup Step

oskosk added [Status] In Progress [Feature] Signup & Account Creation All screens and flows for making a new WordPress.com account. labels Dec 1, 2015

oskosk mentioned this pull request Dec 1, 2015

Signup: Theme step should use ThemesList component #256

Closed

oskosk 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 Dec 1, 2015

scruffian reviewed Dec 1, 2015
View reviewed changes

scruffian added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 1, 2015

scruffian assigned oskosk Dec 1, 2015

oskosk added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Author Reply labels Dec 1, 2015

scruffian force-pushed the update/theme-step-using-theme-list-component branch from 070988e to 2b6f645 Compare December 1, 2015 15:29

scruffian added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 1, 2015

oskosk added 3 commits December 2, 2015 11:00

Use 'Pick' as label for Theme screenshots on hover.

6c9f84c

The scss file for theme-selection is no longer needed and is also removed by this commit.

scruffian force-pushed the update/theme-step-using-theme-list-component branch from 2b6f645 to 6c9f84c Compare December 2, 2015 11:00

scruffian added a commit that referenced this pull request Dec 2, 2015

Merge pull request #1079 from Automattic/update/theme-step-using-them…

f1bf71d

…e-list-component Signup: Replace usage of ThemeThumbnail for ThemesList component on Theme Selection Signup Step

scruffian merged commit f1bf71d into master Dec 2, 2015

scruffian deleted the update/theme-step-using-theme-list-component branch December 2, 2015 11:27

scruffian removed the [Status] Ready to Merge label Dec 2, 2015

This was referenced Dec 2, 2015

Signup: Headstart: Headstart flows should use the ThemesList shared component. #674

Closed

Signup: Theme step: layout is distorted on fluid width #775

Closed

oskosk mentioned this pull request Dec 10, 2015

Signup: move MLB flow to Calypso #541

Closed

10 tasks

anarh mentioned this pull request Nov 28, 2023

[Snyk] Fix for 13 vulnerabilities anarh/wp-calypso#115

Open

mdno mentioned this pull request Nov 28, 2023

[Snyk] Fix for 13 vulnerabilities mdno/wp-calypso#127

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Signup: Replace usage of ThemeThumbnail for ThemesList component on Theme Selection Signup Step #1079

Signup: Replace usage of ThemeThumbnail for ThemesList component on Theme Selection Signup Step #1079

oskosk commented Dec 1, 2015

scruffian Dec 1, 2015

oskosk Dec 1, 2015

folletto Dec 1, 2015

oskosk Dec 1, 2015

scruffian commented Dec 1, 2015

scruffian Dec 1, 2015

oskosk Dec 1, 2015

scruffian commented Dec 1, 2015

folletto commented Dec 1, 2015

oskosk commented Dec 1, 2015

oskosk commented Dec 1, 2015

folletto commented Dec 1, 2015

scruffian commented Dec 1, 2015

scruffian commented Dec 1, 2015

Signup: Replace usage of ThemeThumbnail for ThemesList component on Theme Selection Signup Step #1079

Signup: Replace usage of ThemeThumbnail for ThemesList component on Theme Selection Signup Step #1079

Conversation

oskosk commented Dec 1, 2015

Testing instructions

Regular theme selection

Headstart flow

/design theme selection for existing sites

scruffian Dec 1, 2015

Choose a reason for hiding this comment

oskosk Dec 1, 2015

Choose a reason for hiding this comment

folletto Dec 1, 2015

Choose a reason for hiding this comment

oskosk Dec 1, 2015

Choose a reason for hiding this comment

scruffian commented Dec 1, 2015

scruffian Dec 1, 2015

Choose a reason for hiding this comment

oskosk Dec 1, 2015

Choose a reason for hiding this comment

scruffian commented Dec 1, 2015

folletto commented Dec 1, 2015

oskosk commented Dec 1, 2015

oskosk commented Dec 1, 2015

folletto commented Dec 1, 2015

scruffian commented Dec 1, 2015

scruffian commented Dec 1, 2015