Remove: __experimentalHasMultipleOrigins prop from colors and gradients #46315
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jorgefilipecosta
added
the
[Type] Code Quality
jorgefilipecosta
requested review from
ajitbohra,
ellatrix and
tellthemachines
as code owners
|
Size Change: -291 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
Contributor
|
I came across this PR while researching #46148 regarding the The problem is that when Should this be addressed in the follow-up PR? |
youknowriad
reviewed
Dec 14, 2022
Contributor
youknowriad
left a comment
youknowriad
left a comment
There was a problem hiding this comment.
Looks like there's some related e2e test failures.
| ) { | ||
| return <PanelColorGradientSettingsInner { ...props } />; | ||
| } | ||
| if ( props.__experimentalHasMultipleOrigins ) { |
Contributor
There was a problem hiding this comment.
I don't understand the changes on this component, can you detail a bit more?
For me you had two options here:
- Either you could have forced all the controls to be "multiple" or "single" (you choose here the shape of the colors/gradients prop to provide to the lower level components from the components package)
- Or keep the
__experimentalHasMultipleOriginsprop at this level (block-editor package components) and construct the shape of the palettes depending on this prop, leaving the choice to the consumer of these components.
bph
added
the
Needs Dev Note
Contributor
|
Added the 'needs dev note' label, so it's flagged to get a mention it in the 6.2 Fieldguide. |
jorgefilipecosta
force-pushed
the
remove/__experimentalHasMultipleOrigins-property
branch
from
b5568fc to
e6a2fd9
Compare
youknowriad
approved these changes
Dec 16, 2022
Contributor
youknowriad
left a comment
youknowriad
left a comment
There was a problem hiding this comment.
This LGTM, you might want to rebase the PR per the recent jobs changes.
jorgefilipecosta
force-pushed
the
remove/__experimentalHasMultipleOrigins-property
branch
2 times, most recently
from
efb6e17 to
8468806
Compare
jorgefilipecosta
force-pushed
the
remove/__experimentalHasMultipleOrigins-property
branch
2 times, most recently
from
307f273 to
e4daa4e
Compare
…ient related components.
jorgefilipecosta
force-pushed
the
remove/__experimentalHasMultipleOrigins-property
branch
from
e4daa4e to
a5fe35e
Compare
jorgefilipecosta
deleted the
remove/__experimentalHasMultipleOrigins-property
branch
Member
Author
Hi @bph, we can write a dev note to get more awareness and testing. But with this PR, users of the string __experimentalHasMultipleOrigins will not notice any change. Our code just got smarter and detects when __experimentalHasMultipleOrigins behavior should be used or not without developers needing to pass it. |
Contributor
|
Sorry, catching up through a long queue 😅 Great cleanup work @jorgefilipecosta ! I opened a follow-up PR (#47384) with a few more cleanup tweaks, but nicely done here 🙌 It would be great to see the same treatment applied to the |
ciampo
added
has dev note
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #42994.
This PR removes the __experimentalHasMultipleOrigins from all the color and gradient-related components.
Instead of having this property, we now inspect the contents of colors and gradients to understand its shape.
cc: @ciampo
✍️ Dev Note
The
__experimentalHasMultipleOriginsprop has been removed from theColorPalette,GradientPicker,BorderControlandBorderBoxControlcomponents in the@wordpress/componentspackage.The prop has been replaced by a check in the
ColorPalettecomponent which automatically detects whether the colors passed through thecolorsprop has a single or multiple color origins.