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

Replace rc-color-picker with FluentUI's colorpicker #1005

Merged
merged 1 commit into from May 20, 2021

Conversation

wkalt
Copy link
Contributor

@wkalt wkalt commented May 18, 2021

No description provided.


export enum ButtonShape {
Circle,
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

separate file? just use a flag?

Copy link
Member

Choose a reason for hiding this comment

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

or just buttonShape?: "circle" | "default"

@wkalt
Copy link
Contributor Author

wkalt commented May 19, 2021

@jtbandes @defunctzombie this is ready for a look

@@ -35,7 +37,7 @@ type Config = {
datatype: string;
buttonText: string;
buttonTooltip: string;
buttonColor: string;
buttonColor: Color;
Copy link
Member

Choose a reason for hiding this comment

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

Changing this will not be backwards-compatible with existing saved layouts – we don't have any migration story right now but you might want to either keep this as a string, or do something like string | Color so you can at least guard against old values

@wkalt
Copy link
Contributor Author

wkalt commented May 19, 2021

there's an issue with this around coloring unlabeled markers with the topic tree. Taking a look at that.

Copy link
Member

@jtbandes jtbandes left a comment

Choose a reason for hiding this comment

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

Looks good! Just need to remove rc-color-picker now :) Thanks for taking the time to do some good cleanups and add tests where appropriate.

Replace rc-color-picker with FluentUI's ColorPicker, and split
"ColorPickerForTopicSettings" into a standard new ColorPicker component
for common usage.
@wkalt wkalt force-pushed the wyatt/GH-902/replace-rc-color-picker branch from 1d5aac0 to 0945837 Compare May 20, 2021 20:16
@wkalt wkalt merged commit 8bec485 into main May 20, 2021
@wkalt wkalt deleted the wyatt/GH-902/replace-rc-color-picker branch May 20, 2021 20:41
@jtbandes jtbandes mentioned this pull request Jun 23, 2021
9 tasks
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

2 participants