Skip to content

Conversation

@rutviksavsani
Copy link
Contributor

@rutviksavsani rutviksavsani commented Sep 2, 2024

Fixes: #64739

What?

This PR addresses an issue where changes to the background and text colors were not immediately reflected in the Calendar block due to the server-side rendering. By adding the key prop and handling null values, we force a re-render when color changes are made, ensuring immediate updates in the block editor.

Why?

The problem was that changes to the background and text colors in the Calendar block were not reflected immediately when updated in the admin panel. The use of <ServerSideRender> required a forced re-render to ensure the updated attributes were reflected. By setting the key prop and handling null values, we ensure the block behaves as expected.

How?

  • Introduced the key prop in the <ServerSideRender> component using a combination of backgroundColor and textColor.
  • Handled null or undefined cases by defaulting to 'default' when the color values are not present.
  • This forces a re-render of the <ServerSideRender> component whenever color changes occur.

Testing Instructions

  1. Open a post or page in the block editor.
  2. Insert the Calendar block.
  3. Use the block settings panel to change the background and text colors.
  4. Confirm that the Calendar block reflects the color changes immediately without requiring a refresh.

Testing Instructions for Keyboard

  1. Use the keyboard to navigate to the block settings panel.
  2. Change the background and text colors using the keyboard.
  3. Verify that the color changes are reflected immediately in the Calendar block.

Screenshots or screencast

Edit.Post.Test.Post.for.Calendar.Block.gutenberg.WordPress.webm

@github-actions
Copy link

github-actions bot commented Sep 2, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: rutviksavsani <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: akasunil <[email protected]>
Co-authored-by: aaronrobertshaw <[email protected]>
Co-authored-by: viralsampat90 <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Sep 2, 2024
@github-actions
Copy link

github-actions bot commented Sep 2, 2024

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @rutviksavsani! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@akasunil
Copy link
Member

akasunil commented Sep 3, 2024

Thanks for the contribution! It works as expected.

20240903_224129

@t-hamano t-hamano self-requested a review September 4, 2024 11:31
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

The ServerSideRender component is expected to be re-rendered in response to changes in the attributes prop. If it is not working as expected, there may be some problem with how we are using the ServerSideRender component or with the ServerSideRender component itself. This PR certainly solves the problem, but we should look for the root cause.

The ServerSideRender component bounces a function that fetches data:

const debouncedFetchData = useDebounce( fetchData, 500 );

And if the previous props and the new props are different, this debounced function should be called:

} else if ( ! fastDeepEqual( prevProps, props ) ) {
debouncedFetchData();

But for some reason, it is not called.

As a test, if I make the following change to not debounce the data fetch, the problem will not be reproduced:

diff --git a/packages/server-side-render/src/server-side-render.js b/packages/server-side-render/src/server-side-render.js
index 45a651e8cc..8dd516cd74 100644
--- a/packages/server-side-render/src/server-side-render.js
+++ b/packages/server-side-render/src/server-side-render.js
@@ -196,7 +196,7 @@ export default function ServerSideRender( props ) {
                if ( prevProps === undefined ) {
                        fetchData();
                } else if ( ! fastDeepEqual( prevProps, props ) ) {
-                       debouncedFetchData();
+                       fetchData();
                }
        } );

My guess is that the call to the debounced function might being canceled for some reason, but I have not yet been able to find the cause.

Another reason is that only the Calendar block skips serialization of text and background colors, which might be the cause. For example, the Tag Cloud block also supports color and is rendered via the ServerSideRender component, but color changes are reflected immediately.

cc @aaronrobertshaw

@aaronrobertshaw
Copy link
Contributor

Thanks for the ping 👍

For example, the Tag Cloud block also supports color and is rendered via the ServerSideRender component, but color changes are reflected immediately.

From memory, the reason the Tag Cloud works is that the color styles and classes are applied to the outer block wrapper and filtered from the server-side-rendered attributes. This means the CSS is already present in the editor and targeting the wrapper. When the server-side rendered block is updated the CSS is applied as expected.

FWIW the same could be achieved for the Calendar block by using getColorClassesAndStyles, then applying those to the editor's wrapper and defining some styles to ensure inheritance. In fact, the block instance element styles already pretty much work like this here.

Incomplete diff illustrating the above (don't use, see below)
diff --git a/packages/block-library/src/calendar/edit.js b/packages/block-library/src/calendar/edit.js
index a60a18d8ee..7334068a46 100644
--- a/packages/block-library/src/calendar/edit.js
+++ b/packages/block-library/src/calendar/edit.js
@@ -10,7 +10,10 @@ import { calendar as icon } from '@wordpress/icons';
 import { Disabled, Placeholder, Spinner } from '@wordpress/components';
 import { useSelect } from '@wordpress/data';
 import ServerSideRender from '@wordpress/server-side-render';
-import { useBlockProps } from '@wordpress/block-editor';
+import {
+	useBlockProps,
+	__experimentalGetColorClassesAndStyles as getColorClassesAndStyles,
+} from '@wordpress/block-editor';
 import { store as coreStore } from '@wordpress/core-data';
 import { __ } from '@wordpress/i18n';
 
@@ -34,7 +37,21 @@ const getYearMonth = memoize( ( date ) => {
 } );
 
 export default function CalendarEdit( { attributes } ) {
-	const blockProps = useBlockProps();
+	// Filter out color block support attributes and apply to the wrapper.
+	// This ensures the immediate update of color selections in the editor.
+	const colorProps = getColorClassesAndStyles( attributes );
+	const serverSideAttributes = {
+		...attributes,
+		style: {
+			...attributes?.style,
+			color: undefined,
+		},
+	};
+	const blockProps = useBlockProps( {
+		className: colorProps.className,
+		style: colorProps.style,
+	} );
+
 	const { date, hasPosts, hasPostsResolved } = useSelect( ( select ) => {
 		const { getEntityRecords, hasFinishedResolution } = select( coreStore );
 
@@ -95,7 +112,10 @@ export default function CalendarEdit( { attributes } ) {
 			<Disabled>
 				<ServerSideRender
 					block="core/calendar"
-					attributes={ { ...attributes, ...getYearMonth( date ) } }
+					attributes={ {
+						...serverSideAttributes,
+						...getYearMonth( date ),
+					} }
 				/>
 			</Disabled>
 		</div>
diff --git a/packages/block-library/src/calendar/style.scss b/packages/block-library/src/calendar/style.scss
index 969d1aafec..eb27c44adb 100644
--- a/packages/block-library/src/calendar/style.scss
+++ b/packages/block-library/src/calendar/style.scss
@@ -19,7 +19,7 @@
 		width: 100%;
 		border-collapse: collapse;
 
-		&:where(:not(.has-text-color)) {
+		&:where( :not( .has-text-color ) ) {
 			color: #40464d;
 
 			// Keep the hard-coded border color for backward compatibility.
@@ -40,6 +40,11 @@
 }
 
 // Keep the hard-coded header background color for backward compatibility.
-:where(.wp-block-calendar table:not(.has-background) th) {
+:where( .wp-block-calendar table:not( .has-background ) th ) {
 	background: $gray-300;
 }
+
+.wp-block-calendar .wp-block-calendar table {
+	color: inherit;
+	background-color: inherit;
+}

As @t-hamano notes, it would be best to solve this at its core. That could arguably be refactoring the blocks to JS using REST API :trollface:

Unfortunately, I've run out of time and bandwidth to help debug further but quickly adding some logging to the fetchData function and where debouncedFetchData is called, it appears as though debouncedFetchData is being called when updating colors however it doesn't seem to be calling fetchData as it should. This could be a thread to follow or a red herring 🤷

@t-hamano
Copy link
Contributor

I found that similar problems occur in other blocks. For example, the following problem can be found in the Latest Comments block:

  • Insert a Latest Comments block
  • ✅ Change the font size
  • Save the post and reload the browser
  • ❌ Change the font size: This means that the block wrapper div's styles/classes will be updated, but the server-side rendered block will not be re-rendered and therefore will not have their styles/classes updated.
  • Save the post and reload the browser
  • Font size changes are reflected

Maybe there is a fundamental problem with the ServerSideRender component itself 🙈

101fe6938e6bdfb27651bc0aad903c2e.mp4

@t-hamano
Copy link
Contributor

I believe #69237 is the best approach, so I'm closing this PR.

@rutviksavsani Thank you for your work!

@t-hamano t-hamano closed this Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Block] Calendar Affects the Calendar Block First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Calendar block text color and background color is not change immediately when we try to change it from block settings.

4 participants