Calendar: Prevent attribute styles/classes from printing twice

[ndiego]

Fixes #43652

What?

In the Editor, styles and classes generated by block attributes are generated twice for the Calendar block, once on the block wrapper and again on the server-side rendered component. This PR stops that from happening.

Why?

Up until recently, the Calendar block had no attributes other than className. Therefore, this bug was not much of an issue. However, as we start adding dimension and color support, issues emerge.

In the case of spacing controls, padding and margin get applied twice. Therefore we cannot implement this functionality without first fixing this issue.

How?

Remove the attributes provided to the server-side rendered component.

Testing Instructions

1. Add a Calendar block.
2. Set a custom class
3. Confirm that this custom class is only applied to the wrapper div.

Note there is still an issue where the classname for the block is printed twice.

Screenshots or screencast

I added the custom class table-class to a Calendar block. Before this PR, it is printed twice. Afterward, only on the wrapper div.

Before:

After:

[t-hamano]

I think that if we remove the attributes prop from the ServerSideRender component, we would not be able to dynamically generate styles and markup based on attirbutes, including block support such as Dimension.
For example, #42029 uses attributes to apply a color to a table tag inside a calendar block.

I have addressed my concerns and suggestions in my comments here, but I think we should have a little more discussion about the policy for addressing the addition of support for blocks using the ServiersideRender component.

Also, as mentioned within the comments above, there are five blocks that use the ServersideRender component, and duplicate class names might need to be resolved in all of them.

[andrewserong]

Good questions, thanks for opening the PR @ndiego and for the ideas here @t-hamano!

I was wondering if another option rather than preventing passing attributes to the ServerSideRender component would be to strip the style key from the rest expansion of ...useBlockProps() on the wrapper div? That way we might be able to defer to the ServerSideRender for all block style output when we're using the server side rendering in the editor.

I haven't tried it, and there could be edge cases with that idea, but just thought I'd raise it in case it seems like another viable option.

[andrewserong]

There's a semi-related PR that selectively removed the test-decoration for the post comments count block's editor rendering (#43497) — while that one was to do with preventing style output that affects the placeholder state, we might be able to use a similar approach in this (and other blocks that use ServerSideRender) to skip the style output at the wrapper div level 🤔

[t-hamano]

@ndiego
@andrewserong

I raised the new approach in my comment here in reference to #43497. I would love to hear your thoughts.

[t-hamano]

Based on the discussion that took place in comment #43243, I think the following might be a good approach.

• Don't pass margin and padding with block support to ServersideRender component (it is simpler to remove spacing all together than to limit it to margin alone, which simplifies the code)

• As for className, only give it to the block wrapper div, as per this PR policy

Here is an example code:

//  Attributes passed to ServerSideRender components.
const serverSideRenderAttirbutes = {
	...attributes,
	...getYearMonth( date ),
	// Exclude additional CSS class(es).
	className: undefined,
	// Exclude spacing block support.
	style: {
		...attributes.style,
		spacing: undefined,
	},
};

return (
	<div { ...blockProps }>
		<Disabled>
			<ServerSideRender
				block="core/calendar"
				attributes={ serverSideRenderAttirbutes }
			/>
		</Disabled>
	</div>
);

I think we shuldn't use omit, etc, because I found that #17025 is trying to move away from Lodash.
This code also takes into account the spacing support that will be added in #43654. However, we will need to brush up on this code for color support, which will be added in #42029.

@ndiego
@andrewserong
We are looking forward to your advice.

[ndiego]

Thanks for the feedback @t-hamano, sorry for the delay, been tied up with WCUS prep. I will take a look at this approach this weekend!

[tyxla]

Just closing the loop on the above comment for the omit usage - #17025 has landed and we're no longer using Lodash in the entire repository.

[t-hamano]

Update: The following PRs related to the calendar block have been merged.

• skipBlockSupportAttributes option has been added to the ServerSiderender component to skip the block support attribute: ServerSideRender: add new skipBlockSupportAttributes prop #44491
• Calendar block supported color: Calendar Block: Add color supports and polish styles #42029

Regarding the first point, the skipBlockSupportAttributes option, resets all block support attributes, so it cannot be used with calendar blocks.

Regarding the second point, since __experimentalSkipSerialization is applied to color, only the attributes related to color (text / background) need to be passed to the ServerSideRender component.

To land this PR, how about referencing the implementation of skipBlockSupportAttributes after rebasing with the latest trunk?

Code

// Don't omit textColor and backgroundColor attributes for which serialization is skipped.
const {
	borderColor,
	fontFamily,
	fontSize,
	gradient,
	className,
	...restAttributes
} = attributes;

// Don't omit color style for which serialization is skipped.
const { border, elements, spacing, typography, ...restStyles } =
	attributes?.style || {};

return (
	<div { ...blockProps }>
		<Disabled>
			<ServerSideRender
				block="core/calendar"
				attributes={ {
					...restAttributes,
					style: restStyles,
					...getYearMonth( date ),
				} }
			/>
		</Disabled>
	</div>
);

cc: @aaronrobertshaw