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

[Colorpicker] Add hex/alpha text fields #8294

Closed
wants to merge 16 commits into from

Conversation

ruairiphackett
Copy link
Contributor

@ruairiphackett ruairiphackett commented Feb 9, 2023

WHY are these changes introduced?

Fixes https://github.com/Shopify/polaris-ux-archive/issues/276 [archived issue].

Replacement for #2186

There was some interest by @alex-page in having this work revisited & I had been meaning to rework #2186 when I got a chance.

Some of the work in #2186 was already added in #4152.

This PR adds hex and transparency text fields to the ColorPicker to make colour picking functionality more accessible.

The specific a11y issues it tries to solve are:

  • The component requires the use of the mouse or touch, since it is implemented with mouse-only events on elements that are not natively interactive (elements).
  • The component is skipped over by screen readers, since there are no semantic elements or roles available.

WHAT is this pull request doing?

Before:

https://screenshot.click/10-04-xkgie-n7k55.png

After:

https://screenshot.click/10-05-5qmem-hmbmx.png

Adds a new optional prop textEditor to ColorPicker.

When textEditor is true 2 new text fields are added to the ColorPicker that allow the user to adjust the colour and alpha values by entering text values for each.

There are also some new examples added to the ColorPicker storybook.

Below is a description of allowed values for both fields:

TextPicker

Accepted values:

Text field that allows user to enter hex or rgb values for the colour. The value will be entered when focus moves from the field or the user presses the enter key. The value is then converted and displayed as a 6 digit hex code.

3 & 6 digit hex codes:

  • #000000
  • 000000
  • #000
  • 000

rgb colours:

  • rgb(255,255,255)
Recording Text field that allows user to enter hex or rgb values for the colour. The value will be entered when focus moves from the field or the user presses the enter key.

AlphaField

Text field that allows user to enter a value between 0 and 100 for the colour's alpha value.

Pressing the up and down arrows while this field is in focus will increase and decrease the value by 1 respectively.

Holding the shift key and pressing the up and down arrows while this field is in focus will increase and decrease the value by 5 respectively.

Recording Text field that allows user to enter a value between 0 and 100 for the colour's alpha value. Pressing the up and down arrows while this field is in focus will increase and decrease the value by 1 respectively. Holding the shift key and pressing the up and down arrows while this field is in focus will increase and decrease the value by 5 respectively.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React, {useState} from 'react';

import {ColorPicker, Page} from '../src';

export function Playground() {
  const [color, setColor] = useState({
    hue: 300,
    brightness: 1,
    saturation: 0.7,
    alpha: 0.7,
  });

  return (
    <Page title="Playground">
      <ColorPicker onChange={setColor} color={color} allowAlpha textEditor />
    </Page>
  );
}

🎩 checklist

@github-actions
Copy link
Contributor

github-actions bot commented Feb 9, 2023

size-limit report 📦

Path Size
polaris-react-cjs 218.85 KB (+0.55% 🔺)
polaris-react-esm 139.73 KB (+0.77% 🔺)
polaris-react-esnext 194.98 KB (+0.64% 🔺)
polaris-react-css 41.63 KB (+0.33% 🔺)

@ruairiphackett ruairiphackett marked this pull request as ready for review February 10, 2023 16:06
@ruairiphackett ruairiphackett requested a review from alex-page March 3, 2023 11:29
@ruairiphackett
Copy link
Contributor Author

@alex-page I recreated this from #2186 following this discussion: #2186 (comment)

Apologies for the delay in returning to this work. Hopefully it will still be of use, if not it was still a good learning experience returning to it.

@alex-page
Copy link
Member

alex-page commented Mar 5, 2023

@ruairiphackett this component is challenging and might be better suited as a pattern. The missing UX that you are building is not essential for every use case of the color picker. It's definitely useful but it's a composition of parts to create a solution. Take a look at patterns, we plan to add these as component code alongside our components. https://polaris.shopify.com/patterns

If you do still want to ship this we would have to rewrite the layout using our new layout primitive components. https://polaris.shopify.com/components/layout-and-structure

@acmertz
Copy link
Contributor

acmertz commented Jul 26, 2023

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @acmertz! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

@acmertz acmertz mentioned this pull request Aug 1, 2023
5 tasks
Copy link
Contributor

Hi! We noticed there hasn’t been activity on this PR in a while. After 30 days, it will close automatically.

If it’s still relevant, or you have updates, comment and let us know. And don’t worry, you can always re-open later if needed.

@pama1999
Copy link

pama1999 commented Jan 27, 2024

this needs to be merged asap :) 👍🏽 +1

@alex-page
Copy link
Member

@pama1999 no it doesn't.

@pama1999
Copy link

@pama1999 no it doesn't.

Okey then how about updating the docs of polaris and replacing the deprecated components with the new ones ;)

@@ -36,6 +36,10 @@
"spinnerAccessibilityLabel": "Loading",
"connectedDisclosureAccessibilityLabel": "Related actions"
},
"ColorPicker": {
"textPickerAccessibilityLabel": "HEX color",
Copy link
Contributor

Choose a reason for hiding this comment

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

Localization quality issue found

The following issues may affect the quality of localized translations if they are not addressed:

  • The value HEX color for key Polaris.ColorPicker.textPickerAccessibilityLabel is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

@@ -36,6 +36,10 @@
"spinnerAccessibilityLabel": "Loading",
"connectedDisclosureAccessibilityLabel": "Related actions"
},
"ColorPicker": {
"textPickerAccessibilityLabel": "HEX color",
"alphaFieldAccessibilityLabel": "Alpha"
Copy link
Contributor

Choose a reason for hiding this comment

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

Localization quality issue found

The following issues may affect the quality of localized translations if they are not addressed:

  • The value Alpha for key Polaris.ColorPicker.alphaFieldAccessibilityLabel is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

Copy link
Contributor

Hi! We noticed there hasn’t been activity on this PR in a while. After 30 days, it will close automatically.

If it’s still relevant, or you have updates, comment and let us know. And don’t worry, you can always re-open later if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants