Skip to content

Conversation

@katinthehatsite
Copy link
Contributor

Closes DOTCOM-14636

Proposed Changes

This PR adds a notice for when the granular Sync is disabled (the one in blue on the screenshot):

Screenshot 2025-09-22 at 11 37 53 AM

Testing Instructions

  • Pull the changes from this branch
  • Ensure that you have an Atomic site along with the staging site
  • Apply the following diff:
index eaeb857a347..af74113a2b0 100644
--- a/client/dashboard/sites/staging-site-sync-modal/index.tsx
+++ b/client/dashboard/sites/staging-site-sync-modal/index.tsx
@@ -288,7 +288,9 @@ function StagingSiteSyncModalInner( {
                  )
                : null;
 
-       const shouldDisableGranularSync = ! lastKnownBackupAttempt && ! isLoadingBackupAttempt;
+       // const shouldDisableGranularSync = ! lastKnownBackupAttempt && ! isLoadingBackupAttempt;
+       const shouldDisableGranularSync = false;
+       
        const hasWarning = shouldDisableGranularSync || sqlNode?.checkState === 'checked';
 
        const handleConfirm = () => {
  • Navigate to /v2/sites/{yourAtomicSite
  • Open the Sync modal (you can use Pull from staging option)
  • Observe the blue notice that says that the granular sync will be enabled later

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)?

@katinthehatsite katinthehatsite self-assigned this Sep 22, 2025
@katinthehatsite katinthehatsite requested a review from a team as a code owner September 22, 2025 09:43
@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 Sep 22, 2025
@github-actions
Copy link

github-actions bot commented Sep 22, 2025

) }
{ shouldDisableGranularSync && (
<VStack style={ { marginTop: '20px' } }>
<Notice density="medium">
Copy link
Contributor Author

@katinthehatsite katinthehatsite Sep 22, 2025

Choose a reason for hiding this comment

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

You will notice the different borders in two notice, please ignore them. I will fix the rounded borders on the WooCommerce notice in another PR

The sharper borders are the correct ones according to the design

@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

  • help-center
  • notifications
  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug fix/add-notice-when-granular-sync-is-disabled on your sandbox.

@katinthehatsite katinthehatsite requested a review from a team September 23, 2025 07:14
Copy link
Contributor

@ivan-ottinger ivan-ottinger left a comment

Choose a reason for hiding this comment

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

Thank you for the changes, Kat. The notice is displaying as expected:

Markup on 2025-09-23 at 11:19:28

Two thoughts:

  1. What if we move the notice above the checkboxes?
  2. I think it would be great to have both checkboxes grayed out (since they cannot be unticket). But maybe you thought about changing this in a separate PR (?)

@katinthehatsite
Copy link
Contributor Author

katinthehatsite commented Sep 23, 2025

I think it would be great to have both checkboxes grayed out (since they cannot be unticket). But maybe you thought about changing this in a separate PR (?)

For this bit, it can not be done without CSS because there is no disabled + checked style. There is disabled and checked styles separately and the checked style will take precedence. When I looked it up last time, I saw a request somewhere in wordpress/components for this. So we can do it with CSS?

Alternatively, we can hide these checkboxes completely and just tell the user that the full sync be performed to avoid confusion with checkboxes and just display the notice, what do you think?

@katinthehatsite
Copy link
Contributor Author

For the disabled checkbox, see:

WordPress/gutenberg#59411
WordPress/gutenberg#69726 (PR for this)

@ivan-ottinger
Copy link
Contributor

For this bit, it can not be done without CSS because there is no disabled + checked style. There is disabled and checked styles separately and the checked style will take precedence. When I looked it up last time, I saw a request somewhere in wordpress/components for this.

I see. Thanks for adding the context. I remembered there was something blocking us here, but couldn't ermember what it was.

So we can do it with CSS?

Do you think it could be handled using inline CSS only? (without adding extra style file) If yes, I think that could work out well.

Alternatively, we can hide these checkboxes completely and just tell the user that the full sync be performed to avoid confusion with checkboxes and just display the notice, what do you think?

I think that if we go this route, we would need to also adjust the notices slightly, e.g. to either update the copy in the database warning (as it currently mentions the checkbox that would be hidden), or add extra line to the notice introduced in this PR. In other words, the approach with CSS adjustment seems cleaner to me.

What do you think?

@ivan-ottinger
Copy link
Contributor

For the disabled checkbox, see:

WordPress/gutenberg#59411
WordPress/gutenberg#69726 (PR for this)

Nice. If we go the route of adding custom CSS for now, we could then add one more maintenance task to remove the custom CSS once the Gutenberg PR is merged.

@katinthehatsite
Copy link
Contributor Author

So I looked a bit more into this and this is the result of moving the notice up:

Screenshot 2025-09-24 at 11 27 46 AM

I think even if we mark the checkboxes as disable, this would not look great. I am wondering if instead of showing the files and folders like this with checkboxes, we might hide them completely for this case and add a text/notice about what will be included during the full sync + database warning into the same place. On the top, we could keep the notice about the granular sync being enabled soon. Although this might go further from the design and would result in the two different looks: one for when the File browser is enabled and one for when we hide it which might not be ideal from the UX perspective. What do you think @ivan-ottinger @Marinatsu ?

@ivan-ottinger
Copy link
Contributor

I think even if we mark the checkboxes as disable, this would not look great. I am wondering if instead of showing the files and folders like this with checkboxes, we might hide them completely for this case and add a text/notice about what will be included during the full sync + database warning into the same place. On the top, we could keep the notice about the granular sync being enabled soon. Although this might go further from the design and would result in the two different looks: one for when the File browser is enabled and one for when we hide it which might not be ideal from the UX perspective. What do you think @ivan-ottinger @Marinatsu ?

Thanks for the exploration, Kat. I think that if we could update that notice message to include what happens during the full sync (as you suggest), I think that hiding the checkboxes completely could work.

When it comes to the approach with disabling checkboxes, maybe the look would be better if that Database warning below was hidden as well (considering similar message would be included in the top notice already).

Curious to hear what Marina thinks as well.

@katinthehatsite
Copy link
Contributor Author

We likely won't be adding the second notice in this manner so let's close this and we can open a different PR for handling this case once we recieve an updated design.

@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 Sep 29, 2025
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.

4 participants