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 bug with NaN values #10308

Closed
shopifybradleystaples opened this issue Sep 1, 2023 · 2 comments
Closed

ColorPicker bug with NaN values #10308

shopifybradleystaples opened this issue Sep 1, 2023 · 2 comments
Assignees
Labels
Bug Something is broken and not working as intended in the system.

Comments

@shopifybradleystaples
Copy link

Issue summary

I think I've encountered a bug with the ColorPicker , or perhaps the utility hsbToHex. Using only the HueSlider I'm ending up with a value of #NaNNaNNan. Initially posted in the Polaris slack channel.

Expected behavior

Clicking on only the HueSlider should not output the value of #NaNNaNNan. I would expect it to either choose some default value that's appropriate for that part of the slider or to not output any value until an actual color is chosen in the left main area of the ColorPicker (unsure of the name of this part -- Slidable?).

Actual behavior

Here's a video showing what actually happens. When you click only on the HueSlider and nowhere else, the value of #NaNNanNan gets outputted.

Looks like most recent change to the actual Polaris ColorPicker component was 4 months ago. web's shared ColorPicker > TextPicker had changes on 8/30/2023, but they seem to be translation related, so unconnected?

I have traced it down to some utilities, I think, but none of them have changed recently. Unsure why this is happening now (or maybe it always did?) In ColorEdit we get a value back from the color picker such as {hue: 190.7462686567164, brightness: NaN, saturation: NaN, alpha: 1} and pass that to a utility from Polaris called hsbToHex. That utility is returning the #NaNNanNan value. Internally, hbsToHex calls hsbToRgb, and that is converting the value above to {red: NaN, green: NaN, blue: NaN, alpha: 1} , which is then trying to parse the red/green/blue NaN values.

Steps to reproduce the problem

For a spin instance

I have duplicated this on a color metafield on my staff store in production, but also have a spin instance at https://admin.web.color-picker-bug.bradley-staples.us.spin.dev/store/shop1/products/1/metafields to show it as well.

  1. Start a new spin instance (or use mine/any that already has a color metafield, and skip to step 5.)
  2. Go to Settings > Custom Data. Then select Products under Metafields.
  3. Click Add Definition. Create a new definition with the name "Color Picker Test", and select Color for the Type. You can ignore the other options. Click Save.
  4. Close the Settings modal
  5. Cick on Products. Select whatever product is first to go to the Product Details Page for that product.
  6. After it fully loads, scroll to bottom and look for the Metafields section. Click on the link Show all in top right of that card.
  7. Start with an empty value for the "Color Picker Test" metafield you created. If needed, you can click on the field for that metafield, then click Clear to remove any previous value.
  8. With the metafield empty of any value, click on the field. A color picker should appear in a popover. Click on the vertical HueSlider, but do not click anywhere else.
  9. You should now have #NaNNaNNaN in the value field of the metafield.

For the reduced test case

  1. Open the browser console to see logs.
  2. Click on the TextField. Do not enter a value.
  3. Click on the HueSlider. Do no click anywhere else.
  4. Note that the TextField has a value of #NaNNaNNan

Reduced test case

https://codesandbox.io/s/shopify-polaris-react-typescript-playground-forked-wdmxdh?file=/src/index.tsx

Specifications

  • Are you using the React components? (Y/N): Y
  • Polaris version number: 11.13.0
  • Browser: Chrome Version 116.0.5845.140 (Official Build) (arm64)
  • Device: Macbook M1 Pro 14-inch, 2021
  • Operating System: MacOS 13.5.1 (22G90)

Or run npx envinfo --system --binaries --browsers --npmPackages react,react-dom,@shopify/polaris to provide specifications on your environment including version numbers, browser, device, and operating system.

Paste the results here:

 System:
    OS: Linux 5.19 Ubuntu 22.04.3 LTS 22.04.3 LTS (Jammy Jellyfish)
    CPU: (8) x64 Intel(R) Xeon(R) CPU @ 2.80GHz
    Memory: 16.87 GB / 47.04 GB
    Container: Yes
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.18.1 - ~/.nvm/versions/node/v16.18.1/bin/node
    Yarn: 1.22.19 - ~/.dev/yarn/1.22.19/bin/yarn
    npm: 8.19.2 - ~/.nvm/versions/node/v16.18.1/bin/npm
    Watchman: 4.9.0 - /usr/bin/watchman
  npmPackages:
    @shopify/polaris: 11.13.0 => 11.13.0
    react: ^18.2.0 => 18.2.0
    react-dom: ^18.2.0 => 18.2.0
@kyledurand
Copy link
Member

Can you take this on and open a PR @shopifybradleystaples? I'd be happy to review. I think these utilities were built, or more likely copied, from some resource like 5 years ago. I'd be happy to review

@kyledurand kyledurand added Bug Something is broken and not working as intended in the system. and removed untriaged labels Sep 2, 2023
@pt8o
Copy link
Contributor

pt8o commented Sep 6, 2023

Turns out the issue is on web!
https://github.com/Shopify/web/issues/103391

@pt8o pt8o closed this as completed Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken and not working as intended in the system.
Projects
None yet
Development

No branches or pull requests

3 participants