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.

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

Editor: Open window with previewUrl if it failed #725

Merged
merged 1 commit into from
Dec 11, 2015

Conversation

johngodley
Copy link
Member

In the desktop app window.open( 'about:blank' ) causes issues, and the Jetpack preview fails.

This PR adds a check for _previewWindow. If it doesn't exist when displaying the previewUrl then directly open the window with the previewUrl.

Note: it may be possible to just create the window here in the first place but I am not sure if it is split for a reason

In the desktop app `window.open( 'about:blank' )` fails.

Add a check for the preview window. If it doesn't exist when we have the previewUrl then directly open the window with the preview URL.

It may be possible to just create the window here in the first place but I am not sure if it is split for a reason
@johngodley johngodley added [Status] In Progress [Feature] WordPress Desktop App Features and improvements related to the WordPress Desktop App. labels Nov 25, 2015
@mjangda
Copy link
Member

mjangda commented Nov 25, 2015

it may be possible to just create the window here in the first place but I am not sure if it is split for a reason

I believe it's to get around pop-up blockers. /cc @aduth for a sanity check?

@aduth
Copy link
Contributor

aduth commented Nov 25, 2015

Note: it may be possible to just create the window here in the first place but I am not sure if it is split for a reason

Windows cannot be opened in asynchronous callbacks, as the call must occur in the same call stack as a user interaction (i.e. click). This is why we open a popup when the user clicks and redirect after the save finishes.

@johngodley johngodley 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 Nov 25, 2015
@mjangda mjangda changed the title Open window with previewUrl if it failed Editor: Open window with previewUrl if it failed Dec 3, 2015
@mjangda mjangda added the [Feature] Post/Page Editor The editor for editing posts and pages. label Dec 3, 2015
@mjangda
Copy link
Member

mjangda commented Dec 10, 2015

🚢 🚢 🚢

@mjangda mjangda 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 10, 2015
johngodley added a commit that referenced this pull request Dec 11, 2015
Editor: Open window with previewUrl if it failed
@johngodley johngodley merged commit cb10632 into master Dec 11, 2015
@johngodley johngodley deleted the fix/jetpack-preview-desktop branch December 11, 2015 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Post/Page Editor The editor for editing posts and pages. [Feature] WordPress Desktop App Features and improvements related to the WordPress Desktop App.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants