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

feat: add Palette component #1304

Merged
merged 14 commits into from
Mar 15, 2024
Merged

feat: add Palette component #1304

merged 14 commits into from
Mar 15, 2024

Conversation

Ruminat
Copy link
Contributor

@Ruminat Ruminat commented Feb 2, 2024

DATAUI-2067

Palette — a basic grid with elements you can select/unselect (a grid of checkboxes basically).

image

Can be customized to display anything basically icons/GIFs/images/reactions/colors and so on:

image

@gravity-ui-bot
Copy link
Contributor

Playwright Test Component is ready.

@gravity-ui-bot
Copy link
Contributor

Preview is ready.

@korvin89
Copy link
Contributor

korvin89 commented Feb 7, 2024

Hey @Ruminat! We've discussed ur pr with @amje @ValeraS and have some suggestions for u:

  1. Add separate component Palette with this interface:
interface PaletteOption {
    value: string;
    content: React.ReactNode;
    title?: string; // HTML title
    disabled?: boolean;
}

interface PaletteProps extends DOMProps, QAProps {
    value?: string[];
    defaultValue?: string[];
    onUpdate?: (value: string[]) => void;
    onFocus?: React.FocusEventHandler;
    onBlur?: React.FocusEventHandler;
    disabled?: boolean;
    name?: string;
    options: PaletteOption[];
    size?: ButtonSize;
}

Palette should have fully keyboard support:

  • the first press of the tab key should focus first option in the component
  • the second press of the tab key should take the focus away from the component
  • navigation between options should be managed by arrow keys

Palette should use this hook

  1. Use Palette as a base for EmojiPalette

@Ruminat
Copy link
Contributor Author

Ruminat commented Feb 9, 2024

Add separate component Palette

I'll add the Palette component (next week, I think)

Do we need the EmojiPalette component in uikit then?
It seems that we can simply put a usage example of Palette for EmojiPalette in storybook
But is there a need to make a distinct EmojiPalette component?

@korvin89
Copy link
Contributor

korvin89 commented Feb 9, 2024

Do we need the EmojiPalette component in uikit then? It seems that we can simply put a usage example of Palette for EmojiPalette in storybook But is there a need to make a distinct EmojiPalette component?

Hmm, we probably don't need it. Example in Storybook sounds quite enough

@Ruminat Ruminat changed the title feat(EmojiPalette): added the EmojiPalette component (and EmojiControl) feat(Palette): added the Palette component (and PaletteControl) Feb 13, 2024
@Ruminat
Copy link
Contributor Author

Ruminat commented Feb 13, 2024

Turned the component into Palette

@Ruminat
Copy link
Contributor Author

Ruminat commented Feb 19, 2024

@korvin89 @amje @ValeraS ping

@Ruminat
Copy link
Contributor Author

Ruminat commented Feb 21, 2024

@korvin89 @amje @ValeraS ping

src/components/Palette/Palette.tsx Outdated Show resolved Hide resolved
src/components/Palette/Palette.tsx Show resolved Hide resolved
src/components/Palette/Palette.tsx Outdated Show resolved Hide resolved
src/components/Palette/Palette.tsx Outdated Show resolved Hide resolved
src/components/Palette/PaletteControl.tsx Outdated Show resolved Hide resolved
src/components/Palette/Palette.tsx Outdated Show resolved Hide resolved
src/components/Palette/Palette.tsx Outdated Show resolved Hide resolved
src/components/Palette/Palette.tsx Outdated Show resolved Hide resolved
src/components/Palette/Palette.tsx Show resolved Hide resolved
src/components/Palette/README.md Outdated Show resolved Hide resolved
@Ruminat
Copy link
Contributor Author

Ruminat commented Feb 29, 2024

@korvin89 ping

@korvin89
Copy link
Contributor

@korvin89 ping

@Ruminat Seems pretty good! Lets wait for at least another one review from @ValeraS or @amje

@Ruminat
Copy link
Contributor Author

Ruminat commented Mar 4, 2024

@amje ping

src/components/Palette/utils.ts Show resolved Hide resolved
src/components/Palette/__stories__/Palette.stories.tsx Outdated Show resolved Hide resolved
src/components/Palette/Palette.tsx Outdated Show resolved Hide resolved
src/components/Palette/README.md Outdated Show resolved Hide resolved
src/components/Palette/__stories__/Palette.stories.tsx Outdated Show resolved Hide resolved
Comment on lines 21 to 33
font-size: 16px;
}
&_size_s &__option {
font-size: 18px;
}
&_size_m &__option {
font-size: 20px;
}
&_size_l &__option {
font-size: 24px;
}
&_size_xl &__option {
font-size: 28px;
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 shift the sizes down, they're a bit bigger than they should be. You can check font sizes of Button

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shifted the font sizes down according to the Button's --_--icon-size: 12px/16px/16px/16px/20px
The font sizes of Button are basically the same (except for the xl), so I didn't use them.

src/components/Palette/Palette.tsx Outdated Show resolved Hide resolved
@amje
Copy link
Contributor

amje commented Mar 12, 2024

In RTL mode keyboard navigation is broken

@Ruminat
Copy link
Contributor Author

Ruminat commented Mar 13, 2024

In RTL mode keyboard navigation is broken

Fixed

@Ruminat
Copy link
Contributor Author

Ruminat commented Mar 15, 2024

@amje ping

@amje amje changed the title feat(Palette): added the Palette component (and PaletteControl) feat: add Palette component Mar 15, 2024
@Ruminat Ruminat merged commit e731838 into main Mar 15, 2024
4 checks passed
@Ruminat Ruminat deleted the feature/EmojiPalette-component branch March 15, 2024 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants