Skip to content
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

ColorPalette components: Changed Props from __experimentalIsRenderedInSidebar to isRenderedLeftStart #69376

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

shimotmk
Copy link
Contributor

@shimotmk shimotmk commented Mar 2, 2025

What?

I will correct it according to the comment in the PR below.
#69169 (comment)

Changed Props from __experimentalIsRenderedInSidebar to isRenderedLeftStart

Why?

__experimentalIsRenderedInSidebar is not a good prop name, so change it to isRenderedLeftStart.

The names of the props are based on the placement.

...( isRenderedInSidebar
? {
// When in the sidebar: open to the left (stacking),
// leaving the same gap as the parent popover.
placement: 'left-start',
offset: 34,
}
: {
// Default behavior: open below the anchor
placement: 'bottom',
offset: 8,
} ),
...receivedPopoverProps,

I will migrate __experimentalIsRenderedInSidebar to isRenderedLeftStart for components other than packages/format-library/src/text-color/inline.js in a separate branch.

How?

Testing Instructions

  1. Open a post or page.
  2. Insert a Paragraph block.
  3. Inline text color selection works as before.
  4. Sidebar colors also work as before, and you'll get a deprecation warning in the console.

Copy link

github-actions bot commented Mar 2, 2025

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: shimotmk <[email protected]>
Co-authored-by: mirka <[email protected]>

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

@Mamaduka Mamaduka added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components labels Mar 3, 2025
@mirka
Copy link
Member

mirka commented Mar 5, 2025

Sorry for being unclear here, the problem is not really just about naming, but about allowing customization in a more generic way. So the general idea is for the component to simply accept popover props that will be passed down to the internal popover, rather than having a boolean prop that makes it open in a certain arbitrary placement.

There is a whole group of components that have this problem, so it may be good to first work out an overall strategy in #65581 (cc @WordPress/gutenberg-components).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants