GuidedTours: Frontend preview #4461

ehg · 2016-04-01T01:51:44Z

edits by @mcsf

~~Based off #4391~~
Intends to close #4380

TODO

Might be worth looking into DesignPreview for this.
Properly hide/disable primary when showing preview
Properly position preview relative to sidebar (currently breaks with resizes)
z-index: Position preview below .environment-badge
Be more defensive, codewise, when no site selected
Hide the sidebar at first

Current state

(Previously: #)

To test

Add ?tour=main to any page where a site has been selected
Reload the page, hopefully see a preview and a tour
Play around a bit

mcsf · 2016-04-04T16:28:48Z

Might be worth looking into DesignPreview for this.

Given that DesignPreview hasn't landed yet and that judging from #4140 it seems like it does a whole lot more than just what we need, we should just merge this the way it's done with WebPreview and ?iframe=true&preview=true.

Looping @sirbrillig, who might be interested in this use case.

ehg · 2016-04-04T17:22:05Z

we should just merge this the way it's done with WebPreview and ?iframe=true&preview=true.

Yeah, I had a closer look, and we should be OK with our own approach for the MVP. We could then maybe use the cool layoutFocus approach later.

ehg · 2016-04-05T17:05:34Z

client/components/main/index.jsx

+/**
+ * As apparent below, the idea is simple: connect Main to Redux so it knows
+ * whether it should render a guided tour rather than `primary`.
+ *


Why should Main care about this? Aren't we just overlaying the preview?

Right, I should've made the motivation clear:

The preview is currently overlaid, but if you try resizing you'll see a lot of funkiness going on, as the sidebar shrinks, then hides. Specifically, you'll see a glimpse of primary in between the sidebar and the overlay.

My reasoning, which I should've shared yesterday, is that it's probably a bad idea to rely on a position: fixed element to make sure we fully hide Calypso consistently across different viewports and wild scenarios. Thus, I'd like Main — since it hierarchically sits pretty close to primary (though sadly not above it; suggestions for other components that would work across sections are welcome) — to do the following, roughly speaking:

const classes = classnames( this.props.className, 'main', { // I'm terrible at CSS-oriented naming 'main__is-guidestour-preview-active': shouldShowGuides } ); return ( <main className={ classes } role="main"> { shouldShowGuides && <WebPreview { ...previewProps } /> } { this.props.children } </main> );

That would:

Insert the preview at the same document level of the content / primary, thus gaining more flexibility for good overlay positioning.

Mark <main> with a stateful class so as to make it easier to use plain CSS to hide a Calypso section while the preview is showing.

Is this sound? :)

A chat with @ehg lead to the quick conclusion that, while the general idea likely makes sense, client/layout is a much more suitable component than Main, so I'll get to that.

sirbrillig · 2016-04-05T18:57:51Z

Yeah, I would not rely on DesignPreview yet, although it could definitely be a useful tool for this situation. Things are still too much in flux there. Very cool, though. Thanks for the ping!

mcsf · 2016-04-06T18:08:44Z

@ehg @lsinger — 9a0d87de5cbea035e9132ec26613f73d382143ea is a pretty big and hideous beast, but it switches from touching Main to touching Layout, and implements a bunch of other stuff (handling viewport resizes and also guessing the sidebar width while the UI is still being built for the first time; piping more things through Redux, e.g. killing state.currentStep, etc.).

ehg · 2016-04-06T19:30:03Z

Cool.

According to the mockups, we don't actually need to show the sidebar…

mcsf · 2016-04-06T20:29:50Z

Good point. So the preview could be initialized with left: 0 and only once My Sites is clicked would the preview make room for the sidebar.

ehg · 2016-04-06T20:44:28Z

and only once My Sites is clicked would the preview make room for the sidebar.

Yeah, although I'm kind of wary about leaving the preview there, since there's no current way to get back to this particular form of preview. I think it'll be more harmonious when we have Preview Everywhere, but we could hide the preview once My Sites is clicked now. /cc @shaunandrews

mcsf · 2016-04-06T22:53:23Z

Yeah, although I'm kind of wary about leaving the preview there, since there's no current way to get back to this particular form of preview.

Sorry, I didn't mean closing the preview, I meant, literally, to make room for the sidebar by resizing the preview component so that we can see both.

ehg · 2016-04-07T10:14:51Z

Sorry, I didn't mean closing the preview, I meant, literally, to make room for the sidebar by resizing the preview component so that we can see both.

How about we don't ever show both? :)

ehg · 2016-04-07T11:27:08Z

Flipping this over to design review, so we can nail the UX.

folletto · 2016-04-07T11:50:04Z

The way I could see this working would be:

When ?tour=main is triggered, should instantly start with the preview visible (no fade-in animation) and the masterbar at the top.
All the steps in the sequence will keep the site preview on.
I'd however complete, iterate and gather feedback on the script first. Guided tours text/flow/branching is the most important bit to nail here.

That said, I'm still a bit not comfortable in showing a UI that is never going to be replicated in the current user experience, so my advice would be to wait for layoutPreview to land, and then review this, so moving this part of the UI to a different milestone (M2?).

[Update] Edited the above to clarify, after a chat with Shaun.

- We don't want the preview container to get in the way of the masterbar, so we take the 47px `top` bump from `.web-preview__content` and instead apply it to `.web-preview`. - Hide the current Calypso section if the preview is showing (this is CSS only, the logic to attach stateful classes to the DOM is coming next).

So that we can get rid of state within the GuidesTours component.

mcsf · 2016-04-08T16:46:26Z

Big painful rebase, following #4468, pushed. Now to implement UX changes.

Greatly simplifies component logic w.r.t. calculating, guessing, and keeping track of elements' positions.

We could implement this elsewhere later on.

mcsf · 2016-04-08T18:56:00Z

Experimenting with a full-width preview shown at the start of the tour. Screencast:

mcsf · 2016-04-08T18:59:58Z

Aside from needing a design review, this is good for a code review. (I'll remove the commented-out stuff and squash the commits post-review.)

lsinger · 2016-04-08T19:38:07Z

Awesome!

I usually test by going to e.g. /me?tour-main, and in this case the tour never shows up. Do we need to address that? We're making sure people only land on the proper URLs though, right?

mcsf · 2016-04-08T20:20:05Z

That was intentional, as we need some site to be selected. If we'd like to have the tour operating from /me, we need the site info to e.g. be injected into the GuidesTours state.

lsinger · 2016-04-11T08:43:25Z

That was intentional, as we need some site to be selected. If we'd like to have the tour operating from /me, we need the site info to e.g. be injected into the GuidesTours state.

It would be nice to have it as resilient as possible. Could it make sense (and would it be relatively low effort) to just use the most recently created site (from the set of the user's sites) if none is given?

ehg · 2016-04-11T10:31:56Z

client/components/main/index.jsx

@@ -3,17 +3,17 @@
 /**


Could this go in a separate PR? :P

ehg · 2016-04-11T10:37:12Z

Experimenting with a full-width preview shown at the start of the tour.

I think the design consensus is to show the sidebar over the preview, when it's first opened via a section. Maybe we could get away with manipulating the z-index here? Ref: #4461 (comment) & https://marvelapp.com/c1che6#11469689

ehg · 2016-04-11T10:38:18Z

client/layout/index.jsx

@@ -170,6 +173,8 @@ Layout = React.createClass( {
 					{ this.renderWelcome() }
 					{ this.renderEmailVerificationNotice() }
 					<GlobalNotices id="notices" notices={ notices.list } forcePinned={ 'post' === this.props.section.name } />
+					<GuidesPreview showPreview={ this.props.shouldShowGuidesPreview }


Any reason this has to go in Layout?

IIRC it was motivated by the possibility of moving away from fixed positioning because of how it interacted with the sidebar and primary. It doesn't matter as much now.

shaunandrews · 2016-04-11T15:33:44Z

I dig the direction this is heading. The preview takes a few seconds to load, and you're seeing something like this:

It'd be cool if we had something better to show there. Maybe a blurred out, low-res image of their website. Since this is focused on new users (and we may not have an mShot to use) maybe we could use the theme preview image, at least for Headstart'd sites.? Something like this:

folletto · 2016-04-11T15:40:09Z

maybe we could use the theme preview image, at least for Headstart'd sites

Brilliant idea. 💯

- Note use of createSelector in order to benefit from strict equality checks in GuidesTours#shouldComponentUpdate. - This architecture should make a number of things easier in the future, including adding a preview of the frontend during the tour (#4461).

shaunandrews · 2016-04-12T14:19:20Z

Fun bug where the preview kept reloading new instances within itself:

Maybe related to the private browser window?

ehg · 2016-05-16T14:38:49Z

Closing this for now, in favour of #5117.

ehg self-assigned this Apr 1, 2016

ehg added [Status] In Progress Guided Tours labels Apr 1, 2016

ehg added this to the GuidesTours MVP milestone Apr 1, 2016

ehg assigned mcsf and unassigned ehg Apr 4, 2016

mcsf force-pushed the add/guidestours-frontend-preview branch from 9b0b572 to fbc881c Compare April 4, 2016 16:29

mcsf force-pushed the add/guidestours-frontend-preview branch from fbc881c to 40e5f87 Compare April 5, 2016 17:01

ehg reviewed Apr 5, 2016
View reviewed changes

ehg added the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Apr 7, 2016

mcsf added 8 commits April 8, 2016 17:15

WebPreview: allow passing of style

1dc2059

Add GuidesPreview component, wrapper for WebPreview

78a5845

State: add nextGuidesTourStep & getGuidesTourState

5345f89

So that we can get rid of state within the GuidesTours component.

Guides config: add 'showPreview' flag in steps

4c81c3b

Layout: show GuidesPreview & maybe hide section

6cb45db

GuidesTours: kill component state, connect to state.ui

09df60a

Post-rebase fix

38b2706

mcsf force-pushed the add/guidestours-frontend-preview branch from 1c58390 to 38b2706 Compare April 8, 2016 16:44

mcsf added 2 commits April 8, 2016 19:15

GuidesPreview: take up entire viewport but masterbar

1e37702

Greatly simplifies component logic w.r.t. calculating, guessing, and keeping track of elements' positions.

getGuidesTourState: show as disabled if no site selected

d4c655a

We could implement this elsewhere later on.

mcsf 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 Apr 8, 2016

ehg reviewed Apr 11, 2016
View reviewed changes

client/components/main/index.jsx

@@ -3,17 +3,17 @@

/**

Copy link

Member Author

ehg Apr 11, 2016

Choose a reason for hiding this comment

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

Could this go in a separate PR? :P

ehg reviewed Apr 11, 2016
View reviewed changes

mcsf mentioned this pull request Apr 12, 2016

Guided Tours: Kill component state, use state.ui.guidesTour #4661

Merged

ehg modified the milestones: GuidesTours Future, GuidesTours MVP Apr 12, 2016

ehg added [Status] In Progress and removed [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Apr 25, 2016

lsinger modified the milestones: GuidedTours M2: Preview & Polish, GuidedTours Future Apr 29, 2016

mcsf changed the title ~~GuidesTours: Frontend preview~~ GuidedTours: Frontend preview May 11, 2016

ehg closed this May 16, 2016

matticbot removed the [Status] In Progress label May 16, 2016

lancewillett deleted the add/guidestours-frontend-preview branch May 26, 2016 17:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GuidedTours: Frontend preview #4461

GuidedTours: Frontend preview #4461

ehg commented Apr 1, 2016 •

edited by mcsf

Loading

mcsf commented Apr 4, 2016

ehg commented Apr 4, 2016

ehg Apr 5, 2016

mcsf Apr 5, 2016

mcsf Apr 5, 2016

sirbrillig commented Apr 5, 2016

mcsf commented Apr 6, 2016

ehg commented Apr 6, 2016

mcsf commented Apr 6, 2016

ehg commented Apr 6, 2016

mcsf commented Apr 6, 2016

ehg commented Apr 7, 2016

ehg commented Apr 7, 2016

folletto commented Apr 7, 2016

mcsf commented Apr 8, 2016

mcsf commented Apr 8, 2016

mcsf commented Apr 8, 2016

lsinger commented Apr 8, 2016

mcsf commented Apr 8, 2016 via email

lsinger commented Apr 11, 2016

ehg Apr 11, 2016

ehg commented Apr 11, 2016

ehg Apr 11, 2016

mcsf Apr 12, 2016

shaunandrews commented Apr 11, 2016

folletto commented Apr 11, 2016

shaunandrews commented Apr 12, 2016

ehg commented May 16, 2016

GuidedTours: Frontend preview #4461

GuidedTours: Frontend preview #4461

Conversation

ehg commented Apr 1, 2016 • edited by mcsf Loading

TODO

Current state

To test

mcsf commented Apr 4, 2016

ehg commented Apr 4, 2016

ehg Apr 5, 2016

Choose a reason for hiding this comment

mcsf Apr 5, 2016

Choose a reason for hiding this comment

mcsf Apr 5, 2016

Choose a reason for hiding this comment

sirbrillig commented Apr 5, 2016

mcsf commented Apr 6, 2016

ehg commented Apr 6, 2016

mcsf commented Apr 6, 2016

ehg commented Apr 6, 2016

mcsf commented Apr 6, 2016

ehg commented Apr 7, 2016

ehg commented Apr 7, 2016

folletto commented Apr 7, 2016

mcsf commented Apr 8, 2016

mcsf commented Apr 8, 2016

mcsf commented Apr 8, 2016

lsinger commented Apr 8, 2016

mcsf commented Apr 8, 2016 via email

lsinger commented Apr 11, 2016

ehg Apr 11, 2016

Choose a reason for hiding this comment

ehg commented Apr 11, 2016

ehg Apr 11, 2016

Choose a reason for hiding this comment

mcsf Apr 12, 2016

Choose a reason for hiding this comment

shaunandrews commented Apr 11, 2016

folletto commented Apr 11, 2016

shaunandrews commented Apr 12, 2016

ehg commented May 16, 2016

ehg commented Apr 1, 2016 •

edited by mcsf

Loading