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

Preview: Open preview to previewLink if not autosaveable #7703

Merged
merged 4 commits into from
Jul 6, 2018
Merged

Preview: Open preview to previewLink if not autosaveable #7703

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f757da6
Preview: Open preview to previewLink if not autosaveable
aduth Jul 5, 2018
103f37b
Saving: Update preview button test for interstitial with preview link
aduth Jul 6, 2018
ccd822a
Saving: Avoid unsetting previewWindow on unload event
aduth Jul 6, 2018
6207411
Saving: Compare preview location on reference window object
aduth Jul 6, 2018
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
56 changes: 35 additions & 21 deletions editor/components/post-preview-button/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export class PostPreviewButton extends Component {
constructor() {
super( ...arguments );

this.saveForPreview = this.saveForPreview.bind( this );
this.openPreviewWindow = this.openPreviewWindow.bind( this );
}

componentDidUpdate( prevProps ) {
Expand All @@ -25,7 +25,7 @@ export class PostPreviewButton extends Component {
// This relies on the window being responsible to unset itself when
// navigation occurs or a new preview window is opened, to avoid
// unintentional forceful redirects.
if ( previewLink && previewLink !== prevProps.previewLink ) {
if ( previewLink && ! prevProps.previewLink ) {
this.setPreviewWindowLink( previewLink );
}
}
Expand All @@ -38,48 +38,64 @@ export class PostPreviewButton extends Component {
*/
setPreviewWindowLink( url ) {
const { previewWindow } = this;
if ( ! previewWindow || previewWindow.location.href === url ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: I found that previewWindow.location.href was not returning the value I expected: For the new window, it would return the URL of the editor tab, not the window. (Chrome 67.0.3396.99)

return;
}

previewWindow.location = url;
// Once popup redirect is evaluated, even if already closed, delete
// reference to avoid later assignment of location in a post update.
delete this.previewWindow;

if ( previewWindow && ! previewWindow.closed ) {
previewWindow.location = url;
}
}

getWindowTarget() {
const { postId } = this.props;
return `wp-preview-${ postId }`;
}

saveForPreview( event ) {
const { isDirty, isNew } = this.props;
/**
* Handles a click event to open a popup window and prevent default click
* behavior if the post is either autosaveable or has a previously assigned
* preview link to be shown in the popup window target. Triggers autosave
* if post is autosaveable.
*
* @param {MouseEvent} event Click event from preview button click.
*/
openPreviewWindow( event ) {
const { isAutosaveable, previewLink } = this.props;

// Let default link behavior occur if no changes to saved post
if ( ! isDirty && ! isNew ) {
// If there are no changes to autosave, we cannot perform the save, but
// if there is an existing preview link (e.g. previous published post
// autosave), it should be reused as the popup destination.
if ( ! isAutosaveable && ! previewLink ) {
return;
}

// Save post prior to opening window
this.props.autosave();

// Open a popup, BUT: Set it to a blank page until save completes. This
// is necessary because popups can only be opened in response to user
// interaction (click), but we must still wait for the post to save.
event.preventDefault();
this.previewWindow = window.open(
'about:blank',
isAutosaveable ? 'about:blank' : previewLink,
this.getWindowTarget()
);

if ( ! isAutosaveable ) {
return;
}

this.props.autosave();

const markup = `
<div>
<div class="editor-post-preview-button__interstitial-message">
<p>Please wait&hellip;</p>
<p>Generating preview.</p>
</div>
<style>
body {
margin: 0;
}
div {
.editor-post-preview-button__interstitial-message {
display: flex;
flex-direction: column;
align-items: center;
Expand All @@ -95,10 +111,6 @@ export class PostPreviewButton extends Component {

this.previewWindow.document.write( markup );
this.previewWindow.document.close();

// When popup is closed or redirected by setPreviewWindowLink, delete
// reference to avoid later assignment of location in a post update.
this.previewWindow.onbeforeunload = () => delete this.previewWindow;
}

render() {
Expand All @@ -109,7 +121,7 @@ export class PostPreviewButton extends Component {
className="editor-post-preview"
isLarge
href={ currentPostLink }
onClick={ this.saveForPreview }
onClick={ this.openPreviewWindow }
target={ this.getWindowTarget() }
disabled={ ! isSaveable }
>
Expand All @@ -132,6 +144,7 @@ export default compose( [
isEditedPostDirty,
isEditedPostNew,
isEditedPostSaveable,
isEditedPostAutosaveable,
} = select( 'core/editor' );
const {
getPostType,
Expand All @@ -144,6 +157,7 @@ export default compose( [
isDirty: isEditedPostDirty(),
isNew: isEditedPostNew(),
isSaveable: isEditedPostSaveable(),
isAutosaveable: isEditedPostAutosaveable(),
isViewable: get( postType, [ 'viewable' ], false ),
};
} ),
Expand Down
96 changes: 43 additions & 53 deletions editor/components/post-preview-button/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,6 @@ describe( 'PostPreviewButton', () => {
expect( setter ).not.toHaveBeenCalled();
} );

it( 'should do nothing if the preview window is already at url location', () => {
const url = 'https://wordpress.org';
const setter = jest.fn();
const wrapper = shallow( <PostPreviewButton /> );
wrapper.instance().previewWindow = {
get location() {
return {
href: url,
};
},
set location( value ) {
setter( value );
},
};

wrapper.instance().setPreviewWindowLink( url );

expect( setter ).not.toHaveBeenCalled();
} );

it( 'set preview window location to url', () => {
const url = 'https://wordpress.org';
const setter = jest.fn();
Expand Down Expand Up @@ -80,18 +60,19 @@ describe( 'PostPreviewButton', () => {
isSaveable
modified="2017-08-03T15:05:50" />
);
wrapper.instance().previewWindow = { location: {} };

const previewWindow = { location: {} };

wrapper.instance().previewWindow = previewWindow;

wrapper.setProps( { previewLink: 'https://wordpress.org/?p=1' } );

expect(
wrapper.instance().previewWindow.location
).toBe( 'https://wordpress.org/?p=1' );
expect( previewWindow.location ).toBe( 'https://wordpress.org/?p=1' );
} );
} );

describe( 'saveForPreview()', () => {
function assertForSave( props, isExpectingSave ) {
describe( 'openPreviewWindow()', () => {
function assertForPreview( props, expectedPreviewURL, isExpectingSave ) {
const autosave = jest.fn();
const preventDefault = jest.fn();
const windowOpen = window.open;
Expand All @@ -105,50 +86,59 @@ describe( 'PostPreviewButton', () => {
} );

const wrapper = shallow(
<PostPreviewButton { ...props } autosave={ autosave } />
<PostPreviewButton
postId={ 1 }
{ ...props }
autosave={ autosave }
/>
);

wrapper.simulate( 'click', { preventDefault } );

if ( isExpectingSave ) {
expect( autosave ).toHaveBeenCalled();
if ( expectedPreviewURL ) {
expect( preventDefault ).toHaveBeenCalled();
expect( window.open ).toHaveBeenCalled();
expect( wrapper.instance().previewWindow.document.write ).toHaveBeenCalled();
expect( window.open ).toHaveBeenCalledWith( expectedPreviewURL, 'wp-preview-1' );
} else {
expect( autosave ).not.toHaveBeenCalled();
expect( preventDefault ).not.toHaveBeenCalled();
expect( window.open ).not.toHaveBeenCalled();
}

window.open = windowOpen;

expect( autosave.mock.calls ).toHaveLength( isExpectingSave ? 1 : 0 );
if ( isExpectingSave ) {
expect( wrapper.instance().previewWindow.document.write ).toHaveBeenCalled();
}

return wrapper;
}

it( 'should do nothing if not dirty for saved post', () => {
assertForSave( {
postId: 1,
isNew: false,
isDirty: false,
isSaveable: true,
}, false );
it( 'should do nothing if neither autosaveable nor preview link available', () => {
assertForPreview( {
isAutosaveable: false,
previewLink: undefined,
}, null, false );
} );

it( 'should save if not dirty for new post', () => {
assertForSave( {
postId: 1,
isNew: true,
isDirty: false,
isSaveable: true,
}, true );
it( 'should save for autosaveable post with preview link', () => {
assertForPreview( {
isAutosaveable: true,
previewLink: 'https://wordpress.org/?p=1&preview=true',
}, 'about:blank', true );
} );

it( 'should open a popup window', () => {
assertForSave( {
postId: 1,
isNew: true,
isDirty: true,
isSaveable: true,
}, true );
it( 'should save for autosaveable post without preview link', () => {
assertForPreview( {
isAutosaveable: true,
previewLink: undefined,
}, 'about:blank', true );
} );

it( 'should not save but open a popup window if not autosaveable but preview link available', () => {
assertForPreview( {
isAutosaveable: false,
previewLink: 'https://wordpress.org/?p=1&preview=true',
}, 'https://wordpress.org/?p=1&preview=true', false );
} );
} );

Expand Down
Loading