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.

Sign up for GitHub

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

Render preview in a modal with responsive sizes. #18385

Closed
wants to merge 11 commits into from
Closed

Render preview in a modal with responsive sizes. #18385

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
71d867b
Render preview in a modal with responsive sizes.
tellthemachines Nov 7, 2019
1eed768
Base styling.
tellthemachines Nov 8, 2019
c210afa
Add open in new tab button
tellthemachines Dec 2, 2019
43127d4
Fix for opening in new tab.
tellthemachines Dec 3, 2019
f0b892e
Updated unit tests.
tellthemachines Dec 4, 2019
00b9f49
Updated e2e tests.
tellthemachines Dec 4, 2019
713cbe7
Remove DotTip
tellthemachines Dec 4, 2019
68a7162
Address PR feedback and update tests
tellthemachines Dec 5, 2019
fe836cd
Labelling and keyboard focus improvements
tellthemachines Dec 9, 2019
c66fc1d
Fix scrollbar on frame container
tellthemachines Dec 9, 2019
f3b7e6f
Update snapshot
tellthemachines Dec 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions packages/e2e-tests/specs/editor/various/preview.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ async function openPreviewPage( editorPage ) {
let openTabs = await browser.pages();
const expectedTabsCount = openTabs.length + 1;
await editorPage.click( '.editor-post-preview' );
await editorPage.waitForSelector( '.editor-block-preview__new-tab' );
await editorPage.click( '.editor-block-preview__new-tab' );

// Wait for the new tab to open.
while ( openTabs.length < expectedTabsCount ) {
Expand All @@ -31,9 +33,7 @@ async function openPreviewPage( editorPage ) {
}

const previewPage = last( openTabs );
// Wait for the preview to load. We can't do interstitial detection here,
// because it might load too quickly for us to pick up, so we wait for
// the preview to load by waiting for the title to appear.
// Wait for the preview to load by waiting for the title to appear.
await previewPage.waitForSelector( '.entry-title' );
return previewPage;
}
Expand All @@ -48,6 +48,8 @@ async function openPreviewPage( editorPage ) {
*/
async function waitForPreviewNavigation( previewPage ) {
await page.click( '.editor-post-preview' );
await page.waitForSelector( '.editor-block-preview__new-tab' );
await page.click( '.editor-block-preview__new-tab' );
return previewPage.waitForNavigation();
}

Expand Down Expand Up @@ -117,6 +119,7 @@ describe( 'Preview', () => {

// Return to editor to change title.
await editorPage.bringToFront();
await editorPage.click( '[aria-label="Close dialog"' );
await editorPage.type( '.editor-post-title__input', '!' );
await waitForPreviewNavigation( previewPage );

Expand All @@ -127,12 +130,14 @@ describe( 'Preview', () => {
// Pressing preview without changes should bring same preview window to
// front and reload, but should not show interstitial.
await editorPage.bringToFront();
await editorPage.click( '[aria-label="Close dialog"' );
await waitForPreviewNavigation( previewPage );
previewTitle = await previewPage.$eval( '.entry-title', ( node ) => node.textContent );
expect( previewTitle ).toBe( 'Hello World!' );

// Preview for published post (no unsaved changes) directs to canonical URL for post.
await editorPage.bringToFront();
await editorPage.click( '[aria-label="Close dialog"' );
await publishPost();

// Return to editor to change title.
Expand All @@ -156,6 +161,7 @@ describe( 'Preview', () => {
//
// See: https://github.com/WordPress/gutenberg/issues/7561
await editorPage.bringToFront();
await editorPage.click( '[aria-label="Close dialog"' );
await waitForPreviewNavigation( previewPage );

// Title in preview should match updated input.
Expand Down Expand Up @@ -185,6 +191,7 @@ describe( 'Preview', () => {

// Return to editor.
await editorPage.bringToFront();
await editorPage.click( '[aria-label="Close dialog"' );

// Append bbbbb to the title, and tab away from the title so blur event is triggered.
await editorPage.type( '.editor-post-title__input', 'bbbbb' );
Expand Down Expand Up @@ -238,6 +245,7 @@ describe( 'Preview with Custom Fields enabled', () => {

// Return to editor and modify the title and content.
await editorPage.bringToFront();
await editorPage.click( '[aria-label="Close dialog"' );
await editorPage.click( '.editor-post-title__input' );
await pressKeyWithModifier( 'primary', 'a' );
await editorPage.keyboard.press( 'Delete' );
Expand All @@ -258,5 +266,6 @@ describe( 'Preview with Custom Fields enabled', () => {

// Make sure the editor is active for the afterEach function.
await editorPage.bringToFront();
await editorPage.click( '[aria-label="Close dialog"' );
} );
} );
250 changes: 135 additions & 115 deletions packages/editor/src/components/post-preview-button/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,96 +6,35 @@ import { get } from 'lodash';
/**
* WordPress dependencies
*/
import { Component, renderToString } from '@wordpress/element';
import { Button, Path, SVG } from '@wordpress/components';
import { Component } from '@wordpress/element';
import { Button, IconButton, Modal, Path, SVG } from '@wordpress/components';
import { __, _x } from '@wordpress/i18n';
import { withSelect, withDispatch } from '@wordpress/data';
import { ifCondition, compose } from '@wordpress/compose';
import { applyFilters } from '@wordpress/hooks';

function writeInterstitialMessage( targetDocument ) {
let markup = renderToString(
<div className="editor-post-preview-button__interstitial-message">
<SVG xmlns="http://www.w3.org/2000/svg" viewBox="0 0 96 96">
<Path className="outer" d="M48 12c19.9 0 36 16.1 36 36S67.9 84 48 84 12 67.9 12 48s16.1-36 36-36" fill="none" />
<Path className="inner" d="M69.5 46.4c0-3.9-1.4-6.7-2.6-8.8-1.6-2.6-3.1-4.9-3.1-7.5 0-2.9 2.2-5.7 5.4-5.7h.4C63.9 19.2 56.4 16 48 16c-11.2 0-21 5.7-26.7 14.4h2.1c3.3 0 8.5-.4 8.5-.4 1.7-.1 1.9 2.4.2 2.6 0 0-1.7.2-3.7.3L40 67.5l7-20.9L42 33c-1.7-.1-3.3-.3-3.3-.3-1.7-.1-1.5-2.7.2-2.6 0 0 5.3.4 8.4.4 3.3 0 8.5-.4 8.5-.4 1.7-.1 1.9 2.4.2 2.6 0 0-1.7.2-3.7.3l11.5 34.3 3.3-10.4c1.6-4.5 2.4-7.8 2.4-10.5zM16.1 48c0 12.6 7.3 23.5 18 28.7L18.8 35c-1.7 4-2.7 8.4-2.7 13zm32.5 2.8L39 78.6c2.9.8 5.9 1.3 9 1.3 3.7 0 7.3-.6 10.6-1.8-.1-.1-.2-.3-.2-.4l-9.8-26.9zM76.2 36c0 3.2-.6 6.9-2.4 11.4L64 75.6c9.5-5.5 15.9-15.8 15.9-27.6 0-5.5-1.4-10.8-3.9-15.3.1 1 .2 2.1.2 3.3z" fill="none" />
</SVG>
<p>{ __( 'Generating preview…' ) }</p>
</div>
);

markup += `
<style>
body {
margin: 0;
}
.editor-post-preview-button__interstitial-message {
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
height: 100vh;
width: 100vw;
}
@-webkit-keyframes paint {
0% {
stroke-dashoffset: 0;
}
}
@-moz-keyframes paint {
0% {
stroke-dashoffset: 0;
}
}
@-o-keyframes paint {
0% {
stroke-dashoffset: 0;
}
}
@keyframes paint {
0% {
stroke-dashoffset: 0;
}
}
.editor-post-preview-button__interstitial-message svg {
width: 192px;
height: 192px;
stroke: #555d66;
stroke-width: 0.75;
}
.editor-post-preview-button__interstitial-message svg .outer,
.editor-post-preview-button__interstitial-message svg .inner {
stroke-dasharray: 280;
stroke-dashoffset: 280;
-webkit-animation: paint 1.5s ease infinite alternate;
-moz-animation: paint 1.5s ease infinite alternate;
-o-animation: paint 1.5s ease infinite alternate;
animation: paint 1.5s ease infinite alternate;
}
p {
text-align: center;
font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Oxygen-Sans, Ubuntu, Cantarell, "Helvetica Neue", sans-serif;
}
</style>
`;

/**
* Filters the interstitial message shown when generating previews.
*
* @param {string} markup The preview interstitial markup.
*/
markup = applyFilters( 'editor.PostPreview.interstitialMarkup', markup );

targetDocument.write( markup );
targetDocument.title = __( 'Generating preview…' );
targetDocument.close();
}
const DesktopIcon = <SVG xmlns="http://www.w3.org/2000/svg" width="24" height="24"><Path d="M0 0h24v24H0z" fill="none" /><Path d="M21 2H3c-1.1 0-2 .9-2 2v12c0 1.1.9 2 2 2h7v2H8v2h8v-2h-2v-2h7c1.1 0 2-.9 2-2V4c0-1.1-.9-2-2-2zm0 14H3V4h18v12z" /></SVG>;
const MobileIcon = <SVG xmlns="http://www.w3.org/2000/svg" width="24" height="24"><Path d="M0 0h24v24H0z" fill="none" /><Path d="M17 1.01L7 1c-1.1 0-2 .9-2 2v18c0 1.1.9 2 2 2h10c1.1 0 2-.9 2-2V3c0-1.1-.9-1.99-2-1.99zM17 19H7V5h10v14z" /></SVG>;
const TabletIcon = <SVG xmlns="http://www.w3.org/2000/svg" width="24" height="24"><Path d="M0 0h24v24H0z" fill="none" /><Path d="M21 4H3c-1.1 0-2 .9-2 2v12c0 1.1.9 2 2 2h18c1.1 0 1.99-.9 1.99-2L23 6c0-1.1-.9-2-2-2zm-2 14H5V6h14v12z" /></SVG>;

export class PostPreviewButton extends Component {
constructor() {
super( ...arguments );

this.openPreviewWindow = this.openPreviewWindow.bind( this );
this.state = {
isPreviewOpen: false,
previewSize: {
width: '1200px',
height: '800px',
},
};
this.openPreviewInNewTab = this.openPreviewInNewTab.bind( this );
this.getWindowTarget = this.getWindowTarget.bind( this );
this.setPreviewWindowLink = this.setPreviewWindowLink.bind( this );
this.openPreviewOverlay = this.openPreviewOverlay.bind( this );
this.closePreviewWindow = this.closePreviewWindow.bind( this );
this.setDesktopPreview = this.setDesktopPreview.bind( this );
this.setTabletPreview = this.setTabletPreview.bind( this );
this.setMobilePreview = this.setMobilePreview.bind( this );
}

componentDidUpdate( prevProps ) {
Expand Down Expand Up @@ -128,7 +67,24 @@ export class PostPreviewButton extends Component {
return `wp-preview-${ postId }`;
}

openPreviewWindow( event ) {
openPreviewOverlay() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be good to stick with the name Modal instead of window or overlay 😄

this.setState( { isPreviewOpen: true } );

// If we don't need to autosave the post before previewing, do nothing.
if ( ! this.props.isAutosaveable ) {
return;
}

// Request an autosave. This happens asynchronously and causes the component
// to update when finished.
if ( this.props.isDraft ) {
this.props.savePost( { isPreview: true } );
} else {
this.props.autosave( { isPreview: true } );
}
}

openPreviewInNewTab( event ) {
// Our Preview button has its 'href' and 'target' set correctly for a11y
// purposes. Unfortunately, though, we can't rely on the default 'click'
// handler since sometimes it incorrectly opens a new tab instead of reusing
Expand All @@ -138,59 +94,123 @@ export class PostPreviewButton extends Component {

// Open up a Preview tab if needed. This is where we'll show the preview.
if ( ! this.previewWindow || this.previewWindow.closed ) {
this.previewWindow = window.open( '', this.getWindowTarget() );
this.previewWindow = window.open( '', '_blank' );
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be _blank? It looks like getWindowTarget is still being used elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a funny one. I found that using the window name returned from getWindowTarget caused window.open to focus the iframe in the modal, rather than opening a new window (this although the iframe doesn't have any name set). Switching to _blank solves that problem and if a preview tab is already open it will still be reused, so doesn't seem to make any difference.

}

// Focus the Preview tab. This might not do anything, depending on the browser's
// and user's preferences.
// https://html.spec.whatwg.org/multipage/interaction.html#dom-window-focus
this.previewWindow.focus();

// If we don't need to autosave the post before previewing, then we simply
// load the Preview URL in the Preview tab.
if ( ! this.props.isAutosaveable ) {
this.setPreviewWindowLink( event.target.href );
return;
}
// Load the Preview URL in the Preview tab.
this.setPreviewWindowLink( event.target.href );
}

// Request an autosave. This happens asynchronously and causes the component
// to update when finished.
if ( this.props.isDraft ) {
this.props.savePost( { isPreview: true } );
} else {
this.props.autosave( { isPreview: true } );
}
setDesktopPreview() {
this.setState( {
previewSize: {
width: '1200px',
height: '800px',
},
} );
}

setTabletPreview() {
this.setState( {
previewSize: {
width: '768px',
height: '1024px',
},
} );
}

setMobilePreview() {
this.setState( {
previewSize: {
width: '375px',
height: '812px',
},
} );
}

// Display a 'Generating preview' message in the Preview tab while we wait for the
// autosave to finish.
writeInterstitialMessage( this.previewWindow.document );
closePreviewWindow() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this closes the modal, so the name of the function is a bit confusing.

this.setState( { isPreviewOpen: false } );
}

render() {
const { previewLink, currentPostLink, isSaveable } = this.props;

// Link to the `?preview=true` URL if we have it, since this lets us see
// changes that were autosaved since the post was last published. Otherwise,
// just link to the post's URL.
const href = previewLink || currentPostLink;

return (
<Button
isLarge
className="editor-post-preview"
href={ href }
target={ this.getWindowTarget() }
disabled={ ! isSaveable }
onClick={ this.openPreviewWindow }
>
{ _x( 'Preview', 'imperative verb' ) }
<span className="screen-reader-text">
{
/* translators: accessibility text */
__( '(opens in a new tab)' )
<>
<Button
isLarge
className="editor-post-preview"
disabled={ ! isSaveable }
onClick={ this.openPreviewOverlay }
>
{ _x( 'Preview', 'imperative verb' ) }

<DotTip tipId="core/editor.preview">
Copy link
Contributor

Choose a reason for hiding this comment

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

Just thought I'd double check that the DotTip should be here? My understanding is they've all been removed in master.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uh, rebase gone wrong 😄

{ __( 'Click “Preview” to load a preview of this page, so you can make sure you’re happy with your blocks.' ) }
</DotTip>
</Button>
{ this.state.isPreviewOpen &&
<Modal
Copy link
Contributor

@talldan talldan Dec 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Indentation here doesn't look quite right. Usually these components are indented an extra tab when there's boolean logic.

title={
// translators: Preview dialog title.
Copy link
Contributor

@talldan talldan Dec 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I think the comment should be indented at the same level as the __() function call below.

__( 'Preview' )
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
__( 'Preview' )
_x( 'Preview', 'imperative verb' )

Copy link
Contributor Author

@tellthemachines tellthemachines Dec 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm maybe the modal title shouldn't be "preview"? It doesn't sound right to have a component title as an imperative. I'll change it to "Preview dialog" for now, open to better suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh right, I missed that it's the noun form. I think you can always change it to _x( 'Preview', 'noun' );

}
</span>
</Button>
onRequestClose={ this.closePreviewWindow }
// Needed so the Modal doesn't close when tabbing into the iframe.
shouldCloseOnClickOutside={ false }
className="editor-block-preview"
Copy link
Contributor

Choose a reason for hiding this comment

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

The classname should be made up of the package and component names. I think this would ideally be something like editor-post-preview-button__preview-modal. I notice the button that was here originally also had the wrong class name, it should be editor-post-preview-button.

>
<div className="editor-block-preview__controls">
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<div className="editor-block-preview__controls">
<div className="editor-post-preview__controls">

<IconButton
icon={ DesktopIcon }
onClick={ this.setDesktopPreview }
label={ __( 'Preview desktop screen' ) }
/>
<IconButton
icon={ TabletIcon }
onClick={ this.setTabletPreview }
label={ __( 'Preview tablet screen' ) }
/>
<IconButton
icon={ MobileIcon }
onClick={ this.setMobilePreview }
label={ __( 'Preview phone screen' ) }
/>
<Button
isLarge
className="editor-block-preview__new-tab"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
className="editor-block-preview__new-tab"
className="editor-post-preview__new-tab"

href={ href }
target={ this.getWindowTarget() }
onClick={ this.openPreviewInNewTab }
>
{ _x( 'Open preview in new tab', 'imperative verb' ) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{ _x( 'Open preview in new tab', 'imperative verb' ) }
{ __( 'Open preview in new tab' ) }

</Button>
</div>
<div className="editor-block-preview__frame-container">
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<div className="editor-block-preview__frame-container">
<div className="editor-post-preview__frame-container">

<iframe
className="editor-block-preview__frame"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
className="editor-block-preview__frame"
className="editor-post-preview__frame"

title={ __( 'Responsive preview frame' ) }
src={ href }
style={
{
width: this.state.previewSize.width,
height: this.state.previewSize.height,
}
}
></iframe>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be self-closing?

Suggested change
></iframe>
/>

</div>
</Modal>
}
</>
);
}
}
Expand Down
Loading