Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

Bug fixes to ColorPickerInput #7299

Merged
merged 9 commits into from
Jan 9, 2024
Merged

Conversation

2metres
Copy link
Contributor

@2metres 2metres commented Jan 3, 2024

User-Facing Changes
Input now can recieve and validate color strings + minor styling pass to bring into alignment with our UI

Screen.Recording.2024-01-03.at.11.24.50.am.mov

Description
Previously the input displayed in the sidebar was readonly and only clicking the swatch would let you change the color. Now the inputs and UI picker are all connected

Resolves FG-4534

@2metres 2metres force-pushed the 2metres/FG-4534-color-picker-input branch from 947d5a3 to 9164b8b Compare January 3, 2024 00:30
@2metres

This comment was marked as resolved.

variant="filled"
value={editedValue ? `#${editedValue.replace("#", "")}` : editedValue}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is less than ideal. Could use help unwravelling the useColorPickerControl hook

Copy link
Contributor

@snosenzo snosenzo Jan 4, 2024

Choose a reason for hiding this comment

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

Why not just use:

Suggested change
value={editedValue ? `#${editedValue.replace("#", "")}` : editedValue}
value={editedValue.startsWith("#") ? editedValue : `#${editedValue}`}

?

Alternatively you could always strip out # when setting the editedValue in useColorPickerControl and have it as a decorator maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we need to return undefined when empty so using starts with doesn't work. I tried moving this into the reducer but i couldn't get it to work

Copy link
Contributor

Choose a reason for hiding this comment

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

we need to return undefined when empty

It looks like editedValue is typed as a string, so I don't understand this bit.

I see you're also now returning displayValue (which makes sense if the intent is to offload the hex display to the calling component); that sounds to me like the value you'd want to display as a text label?

@2metres 2metres marked this pull request as ready for review January 4, 2024 01:32
…ColorPickerInput.tsx

Co-authored-by: Sam Nosenzo <sam@foxglove.dev>
…/inputs/ColorPickerInput.tsx"

This reverts commit e064c0c.
@2metres 2metres merged commit 0b47288 into main Jan 9, 2024
14 checks passed
@2metres 2metres deleted the 2metres/FG-4534-color-picker-input branch January 9, 2024 23:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants