Sidebar: use the "preview" layoutFocus to preview the current site #5117

lsinger · 2016-04-29T12:19:34Z

Uses the preview layoutFocus to preview the current site when clicking it in the sidebar:

To test:

Try clicking the site card on different sites (including Jetpack sites), check it works
Try cold refreshes
Make sure switching sites works OK
DISABLE_FEATURES=preview-layout make run — check behaviour is the same as master

mcsf · 2016-05-04T00:02:43Z

Rebased against parent branch's rebase.

lsinger · 2016-05-12T10:12:47Z

Still works fine after rebase.

ehg · 2016-05-12T16:54:13Z

Rebased against master :)

mcsf · 2016-05-13T09:03:40Z

Noting here that, due to the way the backend generates the preview, we're not necessarily showing the user the same view that they would otherwise see via the actual frontend. For instance, private posts aren't shown:

I added a tentative task in this PR's TODO, which you should feel free to slate for later.

ehg · 2016-05-13T11:44:41Z

Added an external link icon in 4fd9547

ehg · 2016-05-13T12:03:58Z

Navigating in the preview doesn't work because of https://github.com/Automattic/wp-calypso/blob/master/client/my-sites/design-preview/index.js#L132

I wonder if this is something we can implement fairly easily.

mcsf · 2016-05-13T12:22:24Z

Sounds fairly doable.

sirbrillig · 2016-05-16T17:41:11Z

client/my-sites/design-preview/index.js

 	if ( ! state.preview || ! state.preview[ selectedSiteId ] ) {
 		return { selectedSiteId };
 	}
 	const { previewMarkup, customizations, isUnsaved } = state.preview[ selectedSiteId ];
 	return {
+		selectedSite,


You'll probably also want to return selectedSite in the early return a few lines up.

mcsf · 2016-05-16T23:31:58Z

@sirbrillig: while trying to fix the occurrence of blank previews, I noticed the implementation of state/preview/reducer and committed c1b5608.

Please let me know if it makes sense or if I missed a detail.

sirbrillig · 2016-05-17T14:25:56Z

@mcsf: oh, thank you. That's something I was mentally trying to get at but it never quite happened. That should help prevent flickering when the preview hasn't changed.

sirbrillig · 2016-05-18T14:51:26Z

I briefly spoke with @lsinger in Slack about what it would look like to load the DesignPreview by URL instead of with markup from the API. Here's the upsides and downsides:

Using the URL means that the preview will reflect exactly what the user sees on the front-end (eg: Typekit fonts), except that the view will be the "logged-in" view, including the masterbar and any other logged-in behaviors. (The API is set up to show the "logged-out" view to show what the site looks like to others.)
If we want to hide the masterbar or anything else, we would need to do so via an addition to the query string in the URL or by including non-calypso javascript in the preview.
Because the preview would be loaded into the iframe from a different domain, there is no way to read or modify the DOM from calypso. Applying customizations would therefore have to be done by including non-calypso javascript in the preview. (Not an issue if you're not applying changes to the preview.)
Since the preview would be a full version of the site, all links will work in the iframe. This means that the user could navigate to a different page, a different site entirely, or even back into calypso, all from _within_ the preview, which is a pretty terrible experience (this is already possible in the editor preview; here's an example of the editor opened within the editor preview: https://cloudup.com/c2zxgUu01Zx)
Because we can't query the document object of the iframe, the onLoad function of DesignPreview/WebPreview won't work without some modifications.

Most of those aren't insurmountable, but they do require some planning. Number 4 is perhaps the most troubling to me because I'm not sure how to "fix" it, although hiding the masterbar sure would help.

folletto · 2016-05-18T14:56:57Z

If we want to hide the masterbar or anything else, we would need to do so via an addition to the query string in the URL or by including non-calypso javascript in the preview.

Note that this is already available: ?demo=true&iframe=true&theme_preview=true (could be refactored to be neater).

It's used by the Theme Showcase's preview and the iOS/Android apps afaik.

ehg · 2016-05-18T15:48:20Z

Note that this is already available: ?demo=true&iframe=true&theme_preview=true (could be refactored to be neater).

AFAIK that only works for the initial view, so any links clicked within the iframe will show the masterbar.

ehg · 2016-05-18T16:09:20Z

I guess we could load something like a more robust version of the following server side, to get around the masterbar issue at least:

(function(){
  window.onload = function() {
    var anchors = document.getElementsByTagName( 'a' );

    for (var i = 0; i < anchors.length; i++) {
        anchors[i].href = anchors[i].href + '?iframe=true&theme_preview=true';
    }
  }
})();

mcsf · 2016-05-23T12:04:27Z

client/layout/guided-tours/wait.js

+	), delay );
+};
+
+export default wait;


I put this little guy inside the Guided Tours directory as a quick fix, but where do you guys think it really belongs?

lodash? ¯_(ツ)_/¯

Should be fine here for now. We can always move it into lib when it's more mature.

ehg · 2016-05-24T11:39:07Z

Removed Jetpack conditionals, as we don't need them now (61e8bd5).
Split externalUrl code into WebPreview: Allow externalUrl to be set for the external link button #5550
Split refresh code into DesignPreview: Refresh iframe the second time the preview is accessed #5551

ehg · 2016-05-24T12:18:39Z

client/my-sites/current-site/index.jsx

@@ -151,7 +152,9 @@ module.exports = React.createClass( {
 						homeLink={ true }
 						enableActions={ true }
 						externalLink={ true }
-						onSelect={ this.trackHomepageClick }
+						onClick={ this.previewSite }
+						onSelect={ this.previewSite }


Do we want onSelect here?

AFAICT onSelect translates into the onTouchTap event? I'd assume we want that.

ehg · 2016-05-24T12:32:36Z

Squished.

lsinger · 2016-05-24T19:06:30Z

client/my-sites/current-site/index.jsx

@@ -104,8 +104,9 @@ module.exports = React.createClass( {
 		);
 	},

-	trackHomepageClick: function() {
+	previewSite: function( event ) {
 		analytics.ga.recordEvent( 'Sidebar', 'Clicked View Site' );


Clicked View Site => Clicked Preview Site?

This event predates this PR, so we don't want to change it.

Oh, right, it's an actual event name—I'd assumed it was just a string representation for easier reading. Yes, in that light it makes total sense to keep!

lsinger · 2016-05-24T19:08:19Z

The tooltip for the site card is Visit "SITE_TITLE". Can we change that to Preview "SITE_TITLE"?

lsinger · 2016-05-24T19:12:18Z

Generally, this works for and looks good to me.

From time to time the preview takes a bit long to load--but I don't think that's something we could make work better.

The "background image for Dyad theme only shows when browser is forced to repaint through scrolling" is still happening for me, but it seems isolated to my machine. Would be thankful if someone else could try to reproduce:

have a site with the Dyad theme and a header image
when you open preview, does the header image show?

ehg · 2016-05-24T19:55:03Z

The tooltip for the site card is Visit "SITE_TITLE". Can we change that to Preview "SITE_TITLE"?

Changed this and the aria text in 24d46a5c5240df0e405da19f843053945c930961 — also, made sure we're not trying to open a preview when preview-layout isn't enabled in config — which should ensure we don't break non-dev envs(!)

lsinger · 2016-05-24T20:45:15Z

which should ensure we don't break non-dev envs

👍 GOOD call, that one.

bisko · 2016-05-25T08:45:31Z

The "background image for Dyad theme only shows when browser is forced to repaint through scrolling" is still happening for me, but it seems isolated to my machine.

Works properly for me! It seems the theme works a bit strange when updating it through Customizer, because if you have featured content and a header image, the one or other are not properly "switched" until a full refresh after saving and publishing the changes.

Code looks good to me! 👍

A few things I noticed while testing:

Is there a need to send the URL query params (?iframe=true&theme_preview=true&frame-nonce=xxxxxx) when clicking the "Open in external window" button, since it opens the site?
Guided Tours interfere with the preview if the user clicks on the preview too quickly:

- Updating the Theme or Theme options through Customizer doesn't update the preview. It requires hard refresh of the whole page.

ehg · 2016-05-25T08:49:27Z

Thanks @bisko :)

the one or other are not properly "switched" until a full refresh after saving and publishing the changes.

Updating the Theme or Theme options through Customizer doesn't update the preview. It requires hard refresh of the whole page.

#5551

Is there a need to send the URL query params (?iframe=true&theme_preview=true&frame-nonce=xxxxxx) when clicking the "Open in external window" button, since it opens the site?

#5550

Guided Tours interfere with the preview if the user clicks on the preview too quickly:

Interesting, that's Trampoline, and that shouldn't be happening.

lsinger added [Status] In Progress [Type] Task Sidebar Guided Tours labels Apr 29, 2016

lsinger mentioned this pull request Apr 29, 2016

GuidedTours: add a site preview step #5120

Closed

3 tasks

mcsf force-pushed the update/sitecard-preview-layoutfocus branch from 90890c0 to d48b89a Compare May 3, 2016 23:43

mcsf force-pushed the update/sitecard-preview-layoutfocus branch from 814c4a4 to 38d373e Compare May 4, 2016 00:04

ehg force-pushed the update/sitecard-preview-layoutfocus branch 2 times, most recently from 0296023 to 8bbbe87 Compare May 11, 2016 14:05

ehg mentioned this pull request May 12, 2016

Layout: Add preview layoutFocus for DesignPreview #4690

Merged

ehg force-pushed the update/sitecard-preview-layoutfocus branch from a5a6d99 to aad9d11 Compare May 12, 2016 16:53

ehg mentioned this pull request May 16, 2016

GuidedTours: Frontend preview #4461

Closed

6 tasks

sirbrillig reviewed May 16, 2016
View reviewed changes

mcsf force-pushed the update/sitecard-preview-layoutfocus branch from 9332f55 to 411661d Compare May 16, 2016 22:13

ehg added this to the GuidedTours M2: Preview & Polish milestone May 17, 2016

lsinger force-pushed the update/sitecard-preview-layoutfocus branch from c1b5608 to 8807ca1 Compare May 18, 2016 07:58

mcsf reviewed May 23, 2016
View reviewed changes

ehg force-pushed the update/sitecard-preview-layoutfocus branch from a4259cb to e441666 Compare May 24, 2016 10:12

ehg mentioned this pull request May 24, 2016

DesignPreview: Switch off markup injection from endpoint #5491

Merged

ehg force-pushed the update/sitecard-preview-layoutfocus branch 2 times, most recently from a2ea2aa to 6b141fc Compare May 24, 2016 11:25

This was referenced May 24, 2016

DesignPreview: Add error handling; misc fixes #5552

Closed

GuidedTours: Add steps for site preview #5553

Merged

ehg reviewed May 24, 2016
View reviewed changes

ehg force-pushed the update/sitecard-preview-layoutfocus branch from 86acbe9 to f866eb4 Compare May 24, 2016 12:32

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 [Type] Task labels May 24, 2016

lsinger reviewed May 24, 2016
View reviewed changes

bisko 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 May 25, 2016

ehg self-assigned this May 25, 2016

use the preview layoutFocus for the current site card in the sidebar

3ac7bd5

ehg force-pushed the update/sitecard-preview-layoutfocus branch from 24d46a5 to 3ac7bd5 Compare May 25, 2016 09:55

ehg merged commit a5defca into master May 25, 2016

ehg deleted the update/sitecard-preview-layoutfocus branch May 25, 2016 09:58

matticbot removed the [Status] Ready to Merge label May 25, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sidebar: use the "preview" layoutFocus to preview the current site #5117

Sidebar: use the "preview" layoutFocus to preview the current site #5117

lsinger commented Apr 29, 2016 •

edited by ehg

Loading

mcsf commented May 4, 2016

lsinger commented May 12, 2016

ehg commented May 12, 2016

mcsf commented May 13, 2016

ehg commented May 13, 2016

ehg commented May 13, 2016

mcsf commented May 13, 2016

sirbrillig May 16, 2016

mcsf commented May 16, 2016

sirbrillig commented May 17, 2016

sirbrillig commented May 18, 2016

folletto commented May 18, 2016

ehg commented May 18, 2016

ehg commented May 18, 2016

mcsf May 23, 2016

lsinger May 23, 2016

ehg May 24, 2016

ehg commented May 24, 2016

ehg May 24, 2016

lsinger May 24, 2016

ehg commented May 24, 2016

lsinger May 24, 2016

ehg May 24, 2016

lsinger May 24, 2016

lsinger commented May 24, 2016

lsinger commented May 24, 2016

ehg commented May 24, 2016

lsinger commented May 24, 2016

bisko commented May 25, 2016 •

edited

Loading

ehg commented May 25, 2016

Sidebar: use the "preview" layoutFocus to preview the current site #5117

Sidebar: use the "preview" layoutFocus to preview the current site #5117

Conversation

lsinger commented Apr 29, 2016 • edited by ehg Loading

mcsf commented May 4, 2016

lsinger commented May 12, 2016

ehg commented May 12, 2016

mcsf commented May 13, 2016

ehg commented May 13, 2016

ehg commented May 13, 2016

mcsf commented May 13, 2016

Choose a reason for hiding this comment

mcsf commented May 16, 2016

sirbrillig commented May 17, 2016

sirbrillig commented May 18, 2016

folletto commented May 18, 2016

ehg commented May 18, 2016

ehg commented May 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehg commented May 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehg commented May 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lsinger commented May 24, 2016

lsinger commented May 24, 2016

ehg commented May 24, 2016

lsinger commented May 24, 2016

bisko commented May 25, 2016 • edited Loading

ehg commented May 25, 2016

lsinger commented Apr 29, 2016 •

edited by ehg

Loading

bisko commented May 25, 2016 •

edited

Loading