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.

Sign up for GitHub

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

Jump to bottom

feat(NcColorPicker): palette now can have color names #4611

Merged
merged 3 commits into from Nov 17, 2023
Merged

feat(NcColorPicker): palette now can have color names #4611

merged 3 commits into from Nov 17, 2023

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Oct 5, 2023 •
edited

☑️ Resolves

Now palette can optionally be not only an array of string ['#fff', '#000'], but also an array or colors with names [{ color: '#000', name: 'black' }, { color: '#fff', name: 'white' }].

Buttons in the picker have value with a name or a fallback 'A color with a HEX value {hex}'.

Also, a11y fix: now the palette is not a set of independent buttons but a radio button group.

🖼️ Screenshots

🏚️ Before 🏡 After
image image

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

@ShGKme ShGKme added enhancement New feature or request 2. developing Work in progress accessibility Making sure we design for the widest range of people possible, including those who have disabilities feature: colorpicker Related to the colorpicker component labels Oct 5, 2023
@ShGKme ShGKme added this to the 8.0.0 milestone Oct 5, 2023
@ShGKme ShGKme self-assigned this Oct 5, 2023
@ShGKme ShGKme marked this pull request as draft October 6, 2023 09:58
@susnux susnux mentioned this pull request Nov 7, 2023
Merged
16 tasks
@susnux susnux modified the milestones: 8.0.0, 8.1.0 Nov 8, 2023
@AndyScherzinger AndyScherzinger modified the milestones: 8.1.0, 8.1.1 Nov 14, 2023
@susnux susnux modified the milestones: 8.1.1, 8.2.0 Nov 14, 2023
@AndyScherzinger AndyScherzinger modified the milestones: 8.2.0, 8.1.1 Nov 17, 2023
@ShGKme
refactor(utils): fix types GenColors
dfc97ae 
- Convert ES5 function constructor to ES2015 class
- Fix JSDoc tyles
- Use `Math.floor(number)` instead of `parseInt(string)` to convert float number to integer

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme force-pushed the feat/color-picker--labeled-colors branch from 2058a9c to baef643 Compare November 17, 2023 14:46
@ShGKme ShGKme changed the title WIP feat(NcColorPicker): add labeled color feat(NcColorPicker): palette now can have color names Nov 17, 2023
@ShGKme
feat(NcColorPicker): palette now can have color names
d203fb1 
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme force-pushed the feat/color-picker--labeled-colors branch from baef643 to d203fb1 Compare November 17, 2023 14:48
@ShGKme ShGKme added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 17, 2023
@ShGKme
fix(NcColorPicker): make palette a radio buttons group
c7c62e0 
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme force-pushed the feat/color-picker--labeled-colors branch from d5ff44f to c7c62e0 Compare November 17, 2023 15:14
@ShGKme ShGKme marked this pull request as ready for review November 17, 2023 15:14
@ShGKme ShGKme modified the milestones: 8.1.1, 8.2.0 Nov 17, 2023
@susnux
Copy link
Contributor

susnux commented Nov 17, 2023

Works but we should label the default palette in a follow up

@susnux susnux merged commit 06c251a into master Nov 17, 2023
17 checks passed
@susnux susnux deleted the feat/color-picker--labeled-colors branch November 17, 2023 16:21
@ShGKme
Copy link
Contributor Author

ShGKme commented Nov 17, 2023

Works but we should label the default palette in a follow up

The problem is that it is generated... Have no idea how to combine generation and naming, except having a list of 1000 color names and search for the nearest :D

@susnux
Copy link
Contributor

susnux commented Nov 17, 2023

The problem is that it is generated.

But it is not exported and only used in two places, both generate a palette of 6 colors (-> 18 colors total).
So we could either drop that function completely or provide a hardcoded list for that 18 colors.

@ShGKme
Copy link
Contributor Author

ShGKme commented Nov 17, 2023

The problem is that it is generated.

But it is not exported and only used in two places, both generate a palette of 6 colors (-> 18 colors total). So we could either drop that function completely or provide a hardcoded list for that 18 colors.

Agree. I'll do it in follow-ups next week:

  • Add example with new feature
  • Add tests for a new feature
  • Name all the colors from the default palette

@AndyScherzinger AndyScherzinger modified the milestones: 8.2.0, 8.1.1 Nov 17, 2023
@susnux susnux mentioned this pull request Nov 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@susnux susnux susnux approved these changes

@JuliaKirschenheuter JuliaKirschenheuter JuliaKirschenheuter approved these changes

@raimund-schluessler raimund-schluessler Awaiting requested review from raimund-schluessler

@julien-nc julien-nc Awaiting requested review from julien-nc

@Pytal Pytal Awaiting requested review from Pytal

@szaimen szaimen Awaiting requested review from szaimen

Assignees

@ShGKme ShGKme

Labels
3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities enhancement New feature or request feature: colorpicker Related to the colorpicker component
Projects
None yet
Milestone
8.2.0
Development

Successfully merging this pull request may close these issues.

[BITV]: Add accessible names for all color buttons inside of color picker
4 participants
@ShGKme @susnux @JuliaKirschenheuter @AndyScherzinger