-
Notifications
You must be signed in to change notification settings - Fork 54
Studio: Make the Publish button open "Push" dialog after coming from WP.com #2288
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @katinthehatsite for implementing this! I reviewed the related PR in Calypso repo, so I will copy my comments and the screenshots here for completeness:
This nicely wraps up the process of publishing a local site to WordPress.com 🙌. I have tested it, and it works as expected. After purchasing the site, it is created as Atomic, and eventually, Studio connects the site and opens the Push modal.
| Studio click on Publish site | Redirects to WP.com | Open in Studio | Push modal opens | Push completes | Site is published in WP.com |
|---|---|---|---|---|---|
![]() |
![]() |
![]() |
![]() |
![]() |
![]() |
I have also tested the additional step of:
Test the same steps with Create a new WordPress.com site from the Sync modal and confirm that once you connect the site, the Push modal does not auto-open
and it works as expected 👌, but I was wondering if that flow should be similar to the Publish site flow? 🤔 Anyways, that can be discussed as a possible follow-up.
I see one Unit Test failing, but after addressing that, this can be progressed. LGTM! ![]()
I am not sure 😅 This was @sejas request to make the difference between these two buttons - |
Yep! I suggested keeping the same behaviour for the
|
@sejas, yes, that's the one I meant. My reasoning here was that if users are creating a site, they probably want to push their local changes, but I agree they can:
So I agree opening the |
You make a valid point. Newly created sites will usually be empty, so the chances that the user wants to push rather than pull are quite high. I think it's worth discussing whether it's better to open the push modal further. |
sejas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it and it works as expected. It would be great to speed up the actions if possible, like connecting and opening the modal. It could be improved in a follow-up issue.
I'm approving the PR, although there are some conflicts and unit tests that need to be fixed before merging.
connect-and-auto-open-push.mp4
Sounds good, I will open a follow up issue. |
📊 Performance Test ResultsComparing dc27c39 vs trunk site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change |
|
I opened the follow up issue for speed improvement here: STU-1173 |







Related issues
Fixes STU-1108
Proposed Changes
This PR ensures that when you click on
Publish sitebutton and come back to WP.com, the WP.com site is not only connected but that we also open aPushmodal.Testing Instructions
SynctabPublish sitebutton displays in the top right cornerPublish sitebuttonSynctab (might be a couple of seconds)Pushmodal is openCreate a new WordPress.com sitefrom theSyncmodal and confirm that once you connect the site, thePushmodal does not auto-openPre-merge Checklist