Skip to content

Conversation

@epeicher
Copy link
Contributor

@epeicher epeicher commented Aug 7, 2025

Resolves DOTCOM-14143

Proposed Changes

  • Move the onSyncStart callback after the pull/push endpoint calls return
    • This is to ensure that we're refetching the sync status only after the sync was started
  • Add useIsMutating hooks to check if the pull/push mutations are pending
  • Disable the Sync button while the mutations are pending

Why are these changes being made?

  • To fix an issue where the Sync dropdown was not updated correctly to Syncing...
CleanShot.2025-08-07.at.16.43.21.mp4

Testing Instructions

  • Apply these changes or open the Calypso Live link below
  • Navigate to a site that has a staging site and click on the Sync dropdown
  • Select either Push or Pull option
  • Select some files and click on Push/Pull
  • Check that the Sync dropdown switches to Syncing... immediately

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • Have you written new tests for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • Have you tested accessibility for your changes? Ensure the feature remains usable with various user agents (e.g., browsers), interfaces (e.g., keyboard navigation), and assistive technologies (e.g., screen readers) (PCYsg-S3g-p2).
  • Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
    • For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

@epeicher epeicher requested a review from a team August 7, 2025 14:44
@github-actions
Copy link

github-actions bot commented Aug 7, 2025

@epeicher epeicher self-assigned this Aug 7, 2025
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 7, 2025
@matticbot
Copy link
Contributor

matticbot commented Aug 7, 2025

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~39 bytes added 📈 [gzipped])

name                parsed_size           gzip_size
staging-site             +140 B  (+0.0%)      +39 B  (+0.0%)
sites-dashboard           +50 B  (+0.0%)      +23 B  (+0.0%)
site-settings             +50 B  (+0.0%)      +23 B  (+0.0%)
site-performance          +50 B  (+0.0%)      +23 B  (+0.0%)
site-monitoring           +50 B  (+0.0%)      +23 B  (+0.0%)
site-logs                 +50 B  (+0.0%)      +23 B  (+0.0%)
plans                     +50 B  (+0.0%)      +23 B  (+0.0%)
overview                  +50 B  (+0.0%)      +23 B  (+0.0%)
hosting                   +50 B  (+0.0%)      +23 B  (+0.0%)
github-deployments        +50 B  (+0.0%)      +23 B  (+0.0%)
domains                   +50 B  (+0.0%)      +23 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~24 bytes added 📈 [gzipped])

name                                parsed_size           gzip_size
async-load-staging-site-sync-modal       +136 B  (+0.0%)      +24 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

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.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

Copy link
Contributor

@gcsecsey gcsecsey left a comment

Choose a reason for hiding this comment

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

This works as described, but I don't think this is a good approach UX-wise. With these changes, while the Sync button isn't disabled, the user can re-open the modal, and send another request using the sync modal.

CleanShot.2025-08-07.at.16.59.23.mp4

What do you think about pre-emptively disabling the Sync button when either the request is sent, or when the sync modal is closed? We can re-enable it if an error is received from the endpoint.

@katinthehatsite
Copy link
Contributor

This works as described, but I don't think this is a good approach UX-wise. With these changes, while the Sync button isn't disabled, the user can re-open the modal, and send another request using the sync modal.

I agree with this. I think it might lead to some conflicting states.

I did not look into this in detail, but why is the issue present in the first place?

@epeicher
Copy link
Contributor Author

epeicher commented Aug 8, 2025

This works as described, but I don't think this is a good approach UX-wise. With these changes, while the Sync button isn't disabled, the user can re-open the modal, and send another request using the sync modal.

I agree with this. I think it might lead to some conflicting states.

Just curious, why do you think this might lead to conflicting states @katinthehatsite?

I did not look into this in detail, but why is the issue present in the first place?

The issue is because the state is stored in the backend and is read here that comes from here, so if we read the state before the endpoint completes, we are reading an incorrect state. So basically we are making sure that we write the state before reading it. I am going to try to use some local state for this use case, but I need to make sure the backend state is taken into account too. cc: @gcsecsey

@epeicher epeicher requested a review from a team as a code owner August 8, 2025 10:00
@epeicher
Copy link
Contributor Author

epeicher commented Aug 8, 2025

@katinthehatsite, @gcsecsey, I have added dd6899e to disable the Sync button immediately. I then need to reset that local state to only depend on the proper backend state. Could you please test it and let me know what you think about it? Thanks!

@arthur791004
Copy link
Contributor

arthur791004 commented Aug 11, 2025

I checked but the issue seems that the stagingSiteSyncState is not updated after the refetch (fetchStagingSiteSyncState) so the isSyncing is always false. I think we should fix this issue instead of introducing another variable to store it. What do you think?

	const { data: stagingSiteSyncState, refetch: fetchStagingSiteSyncState } = useQuery( {
		...stagingSiteSyncStateQuery( productionSiteId ?? 0 ),
		enabled: !! productionSiteId,
		refetchInterval: ( query ) => {
			return isStagingSiteSyncing( query.state.data ) ? 5000 : false;
		},
		refetchIntervalInBackground: true,
	} );

@arthur791004
Copy link
Contributor

I think we should wait for the response from either pullFromStaging or pushToStaging before calling onSyncStart. Or maybe it would be better to move the call to onSyncStart (and maybe onClose) into the onSuccess function.

See

const handleConfirm = () => {
let include_paths = browserCheckList.includeList.map( ( item ) => item.id ).join( ',' );
let exclude_paths = browserCheckList.excludeList.map( ( item ) => item.id ).join( ',' );
if (
shouldDisableGranularSync ||
( filesAndFoldersNodesCheckState === 'checked' && sqlNode?.checkState === 'checked' )
) {
// Sync everything
include_paths = '';
exclude_paths = '';
}
onSyncStart();
if (
( syncType === 'pull' && environment === 'production' ) ||
( syncType === 'push' && environment === 'staging' )
) {
pullFromStaging( { types: 'paths', include_paths, exclude_paths } );
} else {
pushToStaging( { types: 'paths', include_paths, exclude_paths } );
}
onClose();
};

@gcsecsey
Copy link
Contributor

gcsecsey commented Aug 11, 2025

I checked but the issue seems that the stagingSiteSyncState is not updated after the refetch (fetchStagingSiteSyncState) so the isSyncing is always false. I think we should fix this issue instead of introducing another variable to store it. What do you think?

	const { data: stagingSiteSyncState, refetch: fetchStagingSiteSyncState } = useQuery( {
		...stagingSiteSyncStateQuery( productionSiteId ?? 0 ),
		enabled: !! productionSiteId,
		refetchInterval: ( query ) => {
			return isStagingSiteSyncing( query.state.data ) ? 5000 : false;
		},
		refetchIntervalInBackground: true,
	} );

I looked into this, but for me the refetch is working as expected. However, the stagingSiteSyncState takes a while to be set on the backend, even after the mutations are finished. So there's a brief period of time while the Sync button is not disabled.

CleanShot.2025-08-11.at.15.31.46.mp4

I think we should make sure that we're triggering the refetch in the onSuccess handlers of the pushToStaging and the pullFromStaging mutations to fix this.

@gcsecsey
Copy link
Contributor

gcsecsey commented Aug 11, 2025

I think we should fix this issue instead of introducing another variable to store it. What do you think?

@arthur791004 one alternative way to disable the button is using isMutating to check if the mutations are pending. I updated the PR using this approach, and moving the onSyncStart call to the onSuccess handlers as you suggested above. The only drawback to this approach is that I had to redeclare the mutation keys to avoid importing them from the calypso folder. I think this is overall a more readable way than using the state variable, please let me know WDYT.

As per my testing, both approaches are working great from a UX standpoint, so we can proceed using either one.

CleanShot.2025-08-11.at.16.28.51.mp4

@gcsecsey gcsecsey self-assigned this Aug 11, 2025
@gcsecsey gcsecsey dismissed their stale review August 11, 2025 15:31

I took over working on this, so I'll need a review from someone else :)

@matticbot
Copy link
Contributor

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • notifications
  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug dotcom-14143-staging-sites-redesign-sync-button-does-not-update-when on your sandbox.

@arthur791004
Copy link
Contributor

I think we should move handleClose into the onSuccess handlers and display a busy state on the pull/push button while the mutation is in progress. That way, we won’t need to redeclare the mutation keys. What do you think?

@katinthehatsite
Copy link
Contributor

I think we should move handleClose into the onSuccess handlers and display a busy state on the pull/push button while the mutation is in progress. That way, we won’t need to redeclare the mutation keys. What do you think?

This would mean though that we would need to keep the Sync dialog open during sync which would prevent the user from doing anything in the UI while they are waiting.

Another point is that we do close the dialog in Studio while the sync is in progress so it would be nice to maintain consistency there as well.

@arthur791004
Copy link
Contributor

This would mean though that we would need to keep the Sync dialog open during sync which would prevent the user from doing anything in the UI while they are waiting.

I think the API is meant to trigger the sync process, so we’ll need to check the sync status afterward. Am I missing anything?

Copy link
Contributor

@katinthehatsite katinthehatsite left a comment

Choose a reason for hiding this comment

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

With this PR the Sync button does go to the disabled state right away but I noticed that it does briefly flash back to Sync after a couple of seconds and then goes back to Syncing. See the screencase below:

Screen.Recording.2025-08-12.at.10.38.57.AM.mov

@katinthehatsite
Copy link
Contributor

I think the API is meant to trigger the sync process, so we’ll need to check the sync status afterward. Am I missing anything?

Ah yes, I missed that bit. In that case, I think the suggested approach of moving handleClose to onSuccess makes sense 👍

@gcsecsey
Copy link
Contributor

Thanks for your insights @arthur791004 and @katinthehatsite. 🙌 I moved the handleClose to the onSuccess handler and added a busy state for the button on the modal. This also resolves the Sync button being re-enabled for a short period, as this occured after the mutation completed and before the refetch set the disabled state.

CleanShot.2025-08-12.at.11.12.34.mp4

Please take another look at this and let me know what you think.

Copy link
Contributor

@katinthehatsite katinthehatsite left a comment

Choose a reason for hiding this comment

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

The button is working as expected for me now, thanks for fixing this 👍

@gcsecsey gcsecsey merged commit d8d56bb into trunk Aug 12, 2025
11 checks passed
@gcsecsey gcsecsey deleted the dotcom-14143-staging-sites-redesign-sync-button-does-not-update-when branch August 12, 2025 15:18
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 12, 2025
@arthur791004
Copy link
Contributor

Tested and works as expected 👍

paulopmt1 pushed a commit that referenced this pull request Aug 14, 2025
…5112)

* Staging Sites: Notify syncing after completing the endpoint call

* Add the callback to the pull action too

* Disable sync button immediately after starting syncing

* Check inflight mutations to disable Sync button

* close modal after mutation completes

---------

Co-authored-by: Gergely Csécsey <gcsecsey@gmail.com>
paulopmt1 added a commit that referenced this pull request Aug 21, 2025
* Created the initial structure for the the /contact-info route

* Added some initial components to the screen

* Using DataForm for domain Contact details

* Finished the first interface version

* Reading Contact information from /whois endpoint

* Validating data before saving Contact Info

* Update Contact details data using backend API

* Added isDurty state for the save button

* Added basic validation on DataForm

* Getting supported contries list from our API

* Add support to opt-out 60-day option

* Add Save contact info warning

* Add support to State selection from selected country

* Move country and state logic to inside the contact form component

* Fix re-render issue that removed the focus from text input while typing

* Add eslint rule to prevent lodash usage

* Refactor domain-supported contries and states and removed lodash usage

* Move mutate events to contact-form

* My Jetpack: Updating user connection page header and button text. (#105149)

* Substack Import: fix flow bug (#105167)

* Only display error message in the expiry column in the Site Overview (#105168)

* Enable summer special (#105169)

* Site Overview: Implement renew now action (#105160)

* Site Overview: Implement renew now action

* Fix types

* Use replaceAll

* Redirect by window.location.href

* Increase timeout between atomic transfer and redirect (#105174)

* Increase timeout between atomic transfer and redirect

* Remove unnecessary delay

* Connect Refresh: Extract Gravatar Magic Login into its own space (#104863)

* Connect Refresh: Extract Gravatar Magic Login into its own space

* whole lotta love

* migrate styles

* cleanup. fix publicToken use?

* unfix publicToken use?

* cmon. works now

* fix loginUrl

* lots of cleanup

* fix form outlines

* fix navigation to login

* extract inner components into outer scope

* extract inner components into outer scope

* cleanup. address feedback

* cleanup

* Update client/login/magic-login/gravatar/style.scss

Co-authored-by: Welly Shen <hivoid19@gmail.com>

* address feedback. progress on code-validated

* fix interval not resetting

* fix code validation transition

* Update client/login/magic-login/gravatar/index.tsx

Co-authored-by: Welly Shen <hivoid19@gmail.com>

* Update client/login/magic-login/gravatar/index.tsx

Co-authored-by: Welly Shen <hivoid19@gmail.com>

* Update client/login/magic-login/gravatar/index.tsx

Co-authored-by: Welly Shen <hivoid19@gmail.com>

* cleanup redirect

---------

Co-authored-by: Welly Shen <hivoid19@gmail.com>

* Connect Refresh: Migrate Akismet create-account to unified Signup (#105116)

* Fixes the Promote Post version check hook (#105163)

* Allow plugins upload for plans without sftp (#105176)

* Connect Refresh: Migrate VIP create-account to unified Signup (#105132)

* Remove Summer Special install plugin feature from Business and Ecommerce (#105177)

* Updating @wordpress/components and @wordpress/dataviews packages (#105142)

* Updating @wordpress/components and @wordpress/dataviews packages

* Fix plugin icon

* Fix site icon

---------

Co-authored-by: arthur <arthur.chu@automattic.com>

* Fix font size when rendering ZD HTML (#105180)

* Site settings: add missing learn more links (#105150)

* Staging Sites Redesign: Update Tooltip and checkbox disable style (#105158)

* Improve styles

* Ensure we have checkbox

* Fix both disabaled and checked

* Fix tooltip position

* Clean comments

* Update message

* Fix typo

* Break tooltip in two lines

* Allow full sync when selective sync is not disabled

---------

Co-authored-by: Kateryna Kodonenko <kateryna@automattic.com>

* Hosting Dashboard: Add site logs initial dataviews with data (#105118)

* Connect Refresh: Migrate Jetpack Cloud create-account to unified Signup (#105178)

* add null checks (#105165)

* Staging Sites: Notify syncing after completing the endpoint call (#105112)

* Staging Sites: Notify syncing after completing the endpoint call

* Add the callback to the pull action too

* Disable sync button immediately after starting syncing

* Check inflight mutations to disable Sync button

* close modal after mutation completes

---------

Co-authored-by: Gergely Csécsey <gcsecsey@gmail.com>

* Connect Refresh: Migrate Studio create-account to unified Signup (#105133)

* Components: Fix ambiguous `rem()` import (#103385)

* Domains Hosting Dashboard: Create "Add new DNS record" page (#105095)

* (WIP) Create "Add new DNS record" page to Hosting Dashboard

* Create forms to add all supported DNS record types

* Remove some console.logs and refactor some code

* Refactor code splitting the form configuration for each record type in its own file

* Fix field types and add "required" validation

* Add trailing dot automatically for fields that need to be a FQDN

* Use text area component for TXT record data

* Add description field for DNS records

* Remove empty default element from select fields

* Make placeholders translatable

* Reset form data when changing record types

* Navigate to DNS overview page when clicking on "Cancel"

* Remove query that does not belong to this PR

* Refactor DNSRecord type

* Consolidate component in /dns/add/index.tsx

* Rename `AddDNSRecordFormData` to `DNSRecordFormData` and fix form typing

* Remove `isValid` properties from DNS record configs

* Move DNS record configs to types file

* Rename `types.ts` to `dns-record-configs.ts`

* Small refactor

* Replace straight quote mark by a curly quote mark

* Do not translate DNS record types (e.g. A, ALIAS, CNAME)

* Navigate to DNS overview page after adding a record

* Remove translation from CAA tags

* Improve FQDN comment in the data transformation function

* Remove translations from SRV protocol labels

* Translate placeholder strings for DNS records

* Translate page title

* Remove `WPCOMDNSRecord` that was not being used

* Fix type error because ALIAS did not have a `name` property

* Update type names and definitions to align with #105129

* Update type names to keep consistency

* Update mutation function to a more generic one

* Update mutation function to align with #105129

* A4A > Sign up: Show Onboarding tour in WC flow (#105175)

* Jetpack Cloud: skip wpcomJetpackScanAtomicTransfer middleware on Cloud and A4A (#105189)

* Staging Sites: Add separate expand button and allow selecting nodes by clicking on them in the Jetpack Backup FileBrowser (#105161)

* Allow selecting files by clicking on them

* Add separate expand button option to file browser component

* Remove unnecessary min-width

* Correct grammar in a comment

* Fix styling

* Add RTL support for chevrons

* Improve checkbox accessibility

* Adjust accessibility

* Remove tab index from the node button only when `showFileCard` option is disabled

* Add `expandDirectoriesOnClick` prop to control directory expansion behavior in file browser

* Introduce separate `handleExpandButtonClick` handler

* Expansion directories only when `expandDirectoriesOnClick` prop is true

* Simplify checkbox change logic

* Improve contents of

---------

Co-authored-by: Kateryna Kodonenko <kateryna@automattic.com>

* Redirect from deletion banner when the staging site is deleted (#105164)

* redirect from deletion banner when the staging site is deleted

* check if query is undefined

* use calypso/state notices instead of snackbar

* clean up comments

* CalloutOverlay: Fix the overlapping issue (#105195)

* Fix section name in track events (#105196)

* Hosting Dashboard: Add domain glue records DataView (#105184)

* ZD: Fix metadata when creating a ticket (#105183)

* Packages: Add domain-search to tsconfig (#105186)

* Dashboard v2: render expiring purchase in color (#105170)

* Gravatar: Adjust login-related screens for Gravatar-owned services (#105192)

* Gravatar: Adjust login-related screens for Gravatar owned services

* Reuse one more variable

---------

Co-authored-by: Welly Shen <welly.shen@automattic.com>

* Domain Dashboard: Render registered date and renewal CTA (#105188)

* Render the header and registered date

* Render the renew now button if eligible

* Wrap domain name if it doesnt fit in the container

* Use purchase properties

* Revert callback changes

* Hosting dashboard: Add a custom empty state to the site list (#104544)

* Backups Dashboard: add back up now button (#105190)

* Add enqueue and fetch backup functions

* Add siteBackupsQuery

* Add BackupNowButton

* Render BackupNowButton as Backups page header action

* Remove backup up now button icon

* Refactor BackupNowButton: Simplify button content and tooltip handling

- Moved button content and tooltip text into a structured object for better readability and maintainability.
- Updated mutation function to set the enqueue state before calling the backup function.

* Removed unnecessary empty object from the post request

* Refactor backup data layer to entity/collection pattern

* Domains Hosting Dashboard: DNS records list (#105129)

* Add dns list placeholder

* Add basic data

* Implement data view

* Add value column

* Update section header

* Hide DataView header

* Add actions placeholders

* Add action menu placeholder

* Update dns record list - handle protected records

* Move action to a separate file

* Move fields in a separate file

* Add restore default records actions placeholders

* Add logit to enable/disable dns actions

* Work in progress

* Add delete/edit callback

* Minor fixes after rebase

* Fix edit dns route

* Implement domain restore actions

* Fine tuning

* Fix type issue

* Restore params

* Fix type error

* Restore DnsRecordType

* Fix type

* Address PR review comments

* Fix dropdown

* Address PR review comments

* Add more tracking information around creating conversations (#105199)

* Fix double login e2e (#101899)

* Site Overview: Implement change site address action (#105191)

* Site Overview: Implement change site address action

* Fix types

* Update eslint

* Address feedback

* Fix display incorrect wpcom domain

* fix(sites): prevent PreviewSizePicker crash from invalid previewSize … (#105206)

* fix(sites): prevent PreviewSizePicker crash from invalid previewSize values

* Move previewSize sanitization to existing `sanitizeView` function

* Simplify code for updating `previewSize` to a valid value

* Use `undefined` instead of `230` in order to explicitly get the default

* Check whether the previewSize is smaller than the smallest value

* Check whether the previewSize is NaN

---------

Co-authored-by: Arun <arun@arun.blog>
Co-authored-by: arthur <arthur.chu@automattic.com>

* Domains: Create name-servers component (#105062)

* Create name-servers component

* Marketplace Site Selector: Fix highlight styles within component (#105047)

* Fix styles within component

* Improve syntax

* Remove content-fade

* Cleanup comments

* Remove unnecessary overide

* Keep fade in

* Revert reader changes

* Update colors and fade in

* Improve fade in removal

* Ensure the hover state on badges remain the same

* Only apply the overrides for specific badges

* Fix unintended changes

* Fix unintended change

* Remove redundant styles

---------

Co-authored-by: Kateryna Kodonenko <kateryna@automattic.com>

* Add the hosting dashboard domain forwarding add/edit form (#105113)

The Hosting Dashboard add and edit form for Domain Forwarding rules. There are a few follow up tasks that we'll fix

* Newsletter importer: Update billing warning copy (#105203)

* Newsletter importer: Update billing warning copy.

* Connect Refresh: Fix layout issues with Woo DNA/JPC signup (create-account) form (#105156)

* Domain Search: Update experiment name (#105131)

* Domain Search: Update experiment name

* Enable eligibility only for logged in users

---------

Co-authored-by: Luis Felipe Zaguini <luisfelipezaguini@gmail.com>

* Getting supported contries list from our API

* Fix merge conflict

* Add support to State selection from selected country

* Refactor domain-supported contries and states and removed lodash usage

* Remove unnecessary code after merge

* Renamed contact-details directory to domain-contact-details

* Update domain data type and removed types file

* Use contact-form-fields to store them so the main file doesn't get too big

* Final review changes

* Removed unnecessary css files and fixed types

* Readability improvements

* Fix data types

* Improve support links usage

* Improve the usage of queries for validate mutation

* Fix unnecessary type mapping and remove older query

* Fix type error on StateFieldEdit component

* Fix render error during component update

---------

Co-authored-by: Grzegorz Chudzinski-Pawlowski <112354940+grzegorz-cp@users.noreply.github.com>
Co-authored-by: Tony Arcangelini <33258733+arcangelini@users.noreply.github.com>
Co-authored-by: Griffith Chen <griffith.chen@automattic.com>
Co-authored-by: Veselin Nikolov <veselin@automattic.com>
Co-authored-by: arthur791004 <arthur.chu@automattic.com>
Co-authored-by: Claudiu Filip <claudiu.filip@automattic.com>
Co-authored-by: Christos <chriskmnds@gmail.com>
Co-authored-by: Welly Shen <hivoid19@gmail.com>
Co-authored-by: Sebastián Barbosa <sebabarbosa@gmail.com>
Co-authored-by: Omar Alshaker <omar@omaralshaker.com>
Co-authored-by: Ashar Fuadi <ashar.fuadi@automattic.com>
Co-authored-by: katinthehatsite <katerynakodonenko@gmail.com>
Co-authored-by: Kateryna Kodonenko <kateryna@automattic.com>
Co-authored-by: Karen Attfield <karenlattfield@gmail.com>
Co-authored-by: Gergely Csécsey <gergely.csecsey@automattic.com>
Co-authored-by: Roberto Aranda <roberto.aranda@automattic.com>
Co-authored-by: Gergely Csécsey <gcsecsey@gmail.com>
Co-authored-by: Lena Morita <lena@jaguchi.com>
Co-authored-by: leonardost <leonardost@users.noreply.github.com>
Co-authored-by: Yashwin Poojary <yashwinpoojary@gmail.com>
Co-authored-by: Rafael Agostini <rafael.agostini@automattic.com>
Co-authored-by: Ivan Ottinger <ivan.ottinger@automattic.com>
Co-authored-by: Miroslav Mitev <m1r0@users.noreply.github.com>
Co-authored-by: Payton Swick <payton@automattic.com>
Co-authored-by: Welly Shen <welly.shen@automattic.com>
Co-authored-by: Luis Felipe Zaguini <26530524+zaguiini@users.noreply.github.com>
Co-authored-by: Philip Jackson <p-jackson@users.noreply.github.com>
Co-authored-by: Igor Giussani <igor.giussani@automattic.com>
Co-authored-by: Arun <arun@arun.blog>
Co-authored-by: Bogdan Nikolic <bogdan.nikolic87@gmail.com>
Co-authored-by: Kamen Stanev <kamen.stanev@automattic.com>
Co-authored-by: Allison Levine <1689238+allilevine@users.noreply.github.com>
Co-authored-by: Luis Felipe Zaguini <luisfelipezaguini@gmail.com>
@epeicher
Copy link
Contributor Author

I checked but the issue seems that the stagingSiteSyncState is not updated after the refetch (fetchStagingSiteSyncState) so the isSyncing is always false.

Thanks @arthur791004, that was what I tried to describe in my previous comments

I think we should fix this issue instead of introducing another variable to store it. What do you think?

The original issue was tried to be fixed as part of 6837da2 but there is a delay when reading the sync-state endpoint, so if we want to disable the button after closing the modal we should either:

  • pass the result of the sync-state call to the parent component before closing the modal
  • add an intermediate state as I proposed in dd6899e following a pattern similar to an optimistic update, where we assume the sync is happening while the sync-state endpoint is being called.

Please see the following video in production, we can see the button being enabled if we throttle the network

CleanShot.2025-08-26.at.12.27.38.mp4

I don't think this is a high-priority issue, as it is not common to have those slow networks, but I wanted to point that out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants