-
Couldn't load subscription status.
- Fork 51
Untangling prod and staging sites #1929
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
base: trunk
Are you sure you want to change the base?
Conversation
📊 Performance Test ResultsComparing 0f03c1c vs trunk site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change |
src/modules/sync/index.tsx
Outdated
| const isModalOpen = useRootSelector( connectedSitesSelectors.selectIsModalOpen ); | ||
| const { connectedSites } = useConnectedSitesData(); | ||
| const { syncSites, isFetching, refetchSites } = useSyncSitesData(); | ||
|
|
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.
Looks like unintended change :)
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.
This worked well for me 👍 I did not spot any regressions. Also, the code looked good on my end and I don't have any further concerns:
Two questions that I had were:
- is this change something that we want to ship behind the feature flag or should this be available right away to the user?
- I am finding the grouping of sites a bit messy in the modal to connect sites now that they are separate. I am wondering if we should improve how the sites are organized in there e.g. perhaps first showing all production sites and then all staging sites etc. It does not need to be handled as a part of this PR though, just something to consider further in this project 👍
Yep, we are going to ship it w/o feature flag.
I was also thinking that we should reconsider the design, for example, we can make sections narrower, at least remove the gray line between the site name and the URL, since now it makes feeling like it's 2 different items. But as you menthioned and I agree - it should be a separate issue. This PR is preety big, so I would keep it focused on one thing - untangling. |
Sounds good, we can open a separate issue to handle this and to ask for design input 👍 |
|
@katinthehatsite created issue in Linear STU-918 |
075c5ec to
bb5f445
Compare
Fixes STU-907
Proposed Changes
We want to display each site separately in the Sync dialog, so we no longer group production and staging together. We need this to be consistent with the "create site from wpcom" stepper.
Currently WPcom sites group production and staging sites under the same row. So with this PR we split them, so each row has only a site. That's the current behaviour for Pressable sites.
Also, I analyzed the code, and I don't see the necessity to have any migration. We just stopped using
stagingSiteIds, and they will be cleaned up automatically over time.Testing Instructions
trunk(don't apply this patch)git checkout add/untanglingProdAndStagingSites