ColorPalette: show Clear button even when colors array is empty
#46001
Conversation
This file contains 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
ciampo
added
[Type] Bug
|
|
ciampo
changed the title
ColorPalette: show Clear button even when colors array is empty
tyxla
approved these changes
Nov 23, 2022
|
Note that the failing check is not related to this PR and I've filed a fix for it in #45999. |
tyxla
reviewed
Nov 23, 2022
mirka
reviewed
Nov 23, 2022
Comment on lines
48
to
56
| const asColorObject = ( args.colors as ColorObject[] )?.[ 0 ].color; | ||
| let asPaletteObject; | ||
|
|
||
| if ( ( args.colors as PaletteObject[] )[ 0 ].colors?.length ) { | ||
| asPaletteObject = ( args.colors as PaletteObject[] )[ 0 ] | ||
| .colors[ 0 ].color; | ||
| } | ||
|
|
||
| initialColor = asColorObject || asPaletteObject; |
There was a problem hiding this comment.
I personally find it more readable if we just added some question marks to the original code — what do you think?
const initialColor =
( args.colors as ColorObject[] )?.[ 0 ]?.color ||
( args.colors as PaletteObject[] )?.[ 0 ]?.colors[ 0 ].color;FWIW I'm also fine with removing this "initial color" selection logic altogether. It's not part of the default component logic, and it's probably not a common use case either.
There was a problem hiding this comment.
I'd rather remove it altogether, to be honest — done in 5be28fb
…se less US-centric colors
…ors` is an empty array
ciampo
force-pushed
the
fix/color-palette-clear-button
branch
from
66fe642
to
5be28fb
Compare
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.
What?
Fixes #45928 by showing the
Clearbutton inColorPaletteeven when the array ofcolorsis empty.Why?
Fixing a regression introduced in #44632
How?
CircularOptionPicker, which was effectively the source of the regressioncolors=[]Testing Instructions
ColorPalettecomponent to TypeScript #44632 — the bug should not be reproducibleScreenshots