polaris icon indicating copy to clipboard operation
polaris copied to clipboard

[RangeSlider] Add magic tone

Open matallo opened this issue 1 year ago • 5 comments

WHY are these changes introduced?

This PR introduces a new tone prop with a possible value "magic" for determining the values in the component have been generated by AI. It uses the --p-color-bg-fill-magic token.

Screenshot 2024-01-18 at 8 06 59 PM

references https://github.com/Shopify/polaris/issues/10152

WHAT is this pull request doing?

This is a followup of https://github.com/Shopify/polaris/pull/10819 and follows the pattern suggested by the Polaris team.

How to 🎩

In Storybook

http://localhost:6006/?path=/story/all-components-rangeslider--magic

🖥 Local development instructions 🗒 General tophatting guidelines 📄 Changelog guidelines

🎩 checklist

matallo avatar Jan 18 '24 19:01 matallo

@matallo what is stopping you from adding RangeSlider to magic-ui and doing a CSS override? Does this need to be in Polaris ASAP? Are 3P app developers going to need these features?

alex-page avatar Jan 18 '24 20:01 alex-page

@matallo what is stopping you from adding RangeSlider to magic-ui and doing a CSS override? Does this need to be in Polaris ASAP? Are 3P app developers going to need these features?

@alex-page, the same prop got added to several Polaris components already, including TextField, Checkbox, RadioButton, Select and ChoiceList.

At the moment I don't think 3P app developers are going to need this feature. In general, do we consider it a bad practice to add a prop to Polaris for something that's required in the admin, but not for 3P app developers?

mathildebuenerd avatar Jan 19 '24 08:01 mathildebuenerd

what is stopping you from adding RangeSlider to magic-ui and doing a CSS override?

That can be a solution, we're doing something similar for RichTextEditor. As @mathildebuenerd mentioned I'm following the same pattern as prior components with tone="magic" but there's a wider discussion to be had.

Does this need to be in Polaris ASAP? Are 3P app developers going to need these features?

I don't think so immediately, but I'll let the team using this to answer. Currently the feature is behind a beta flag for the Admin if I'm not wrong.

matallo avatar Jan 22 '24 21:01 matallo

Localization quality issues found

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

  • The value Clear for key Polaris.ActionList.SearchField.clearButtonLabel is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.
  • The value Search for key Polaris.ActionList.SearchField.search is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.
  • The value Search actions for key Polaris.ActionList.SearchField.placeholder 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.