-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Staging sites: Add Delete staging site
button to the Staging Site
tab (when managing staging site)
#98416
base: trunk
Are you sure you want to change the base?
Staging sites: Add Delete staging site
button to the Staging Site
tab (when managing staging site)
#98416
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~265 bytes added 📈 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
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.
@ivan-ottinger, I know this is still in progress but so far it's looking great.
Did you try navigating to /sites
or /overview/productionSite?.url
? maybe that will fix the current bug and improve the UX.
client/sites/tools/staging-site/components/staging-site-card/staging-site-production-card.tsx
Show resolved
Hide resolved
navigate( `/staging-site/${ urlToSlug( productionSite?.url || '' ) }` ); | ||
}, 1000 ); |
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.
What if we navigate to /sites
to list all the sites?
Or at least to /overview/productionSite?.url
?
I think navigating to any of those will improve the UX as switching to production site and staying in the same tab can be confusing.
This could also "auto" fix the issue of displaying the just removed staging site.
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.
That's actually an interesting approach that I haven't explored before.
The idea I had in mind was to navigate the user back to the Staging tab of the Production site where the deletion progress can be observed and the user can spin up a new staging site right away if they wish.
But as you say, navigating the user to the Overview of the production might be actually better UX. I will explore the option. Thanks!
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.
It makes sense to review the alternative approach, but using the production site's staging site tab to show the progress indicator and immediately allowing the users to add a new staging site is quite a good UX.
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.
After further troubleshooting and investigation I have found out that the issue was caused by the existing periodic query invalidation in useDeleteStagingSite
being interrupted during the navigation from the Staging to the Production page.
This also made me think about the issue from a slightly different angle. I have now implemented a simpler solution where we will let the Production page handle the whole deletion process itself. In that proposed change we won't be initiating the deletion from the Staging page at all.
Thank you for the review and suggestions, Antonio! Yes, I marked it as |
Resolves https://github.com/Automattic/dotcom-forge/issues/9974.
Proposed Changes
In this PR I am proposing a solution that adds
Delete staging site
button to theStaging site
tab when we are managing that specific staging site:The proposed solution works as follows:
Delete staging site
button with confirmation modal.deleteStagingSite
value is stored in the session storage and the user is navigated to theStaging Site
tab on the Production site page.deleteStagingSite
value from the session storage is checked and deleted (as it is no longer necessary) and the staging site deletion is triggered.Why are these changes being made?
Testing Instructions
Staging Site
tab of the staging site and try to delete the staging site.Staging Site
tab of the production site.Pre-merge Checklist