-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add numeric property to Text, right align numeric IndexTable cells and titles and add best practice documentation for numeric values in tables #8448
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
Conversation
0a727d7
to
40ce6db
Compare
/** HTML id attribute */ | ||
id?: string; | ||
/** Render OpenType font characters with consistent height and width */ | ||
monospace?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to call this tabular
since that seems like the intended use case? I wonder if calling it monospace
might create confusion with <InlineCode>
which displays a monospace font family.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @aveline. That's an interesting point. I understand the potential for confusion and reasoning behind changing the prop name. My only reservation in making the change is the expectation that the term "monospace" will be used to describe the requirement, as it was on the ticket to introduce the formatting. I think the benefit of the semantic consistency between the prop name and the way it's described might outweigh any potential confusion. @alex-page what are your thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would call this numeric
as it's applying font-variant-numeric
with the values we recommend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alex-page - I'm late to this party, apologies. Why are we using font-variant-numeric
rather than font-variant-mono
. I don't see the numeric variant in our font tokens and I figure we should always try to back our components with available token values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This update along with the rationale in the issue makes sense to me 👍 ✅
polaris.shopify.com/pages/examples/index-table-small-screen-with-all-of-its-elements.tsx
Outdated
Show resolved
Hide resolved
e024fd2
to
fe45652
Compare
…olaris into right-align-indextable
size-limit report 📦
|
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @shopify/polaris-migrator@0.15.0 ### Minor Changes - [#8229](#8229) [`e21fd8d79`](e21fd8d) Thanks [@aveline](https://github.com/aveline)! - Created migration to rename `Card` to `LegacyCard` ### Patch Changes - Updated dependencies \[[`6c0dda128`](6c0dda1)]: - @shopify/polaris-tokens@6.7.0 - @shopify/stylelint-polaris@8.0.1 ## @shopify/polaris@10.33.0 ### Minor Changes - [#8514](#8514) [`78d686db5`](78d686d) Thanks [@lgriffee](https://github.com/lgriffee)! - Add additional changes for migrating `legacy` custom properties from `v10` to `v11` - [#8512](#8512) [`b77c1fe51`](b77c1fe) Thanks [@fatimasajadi](https://github.com/fatimasajadi)! - Add onMouseEnter prop to the each item on the ActionList component - [#8530](#8530) [`bec50d4d8`](bec50d4) Thanks [@kyledurand](https://github.com/kyledurand)! - Removed alignment defaults from Inline and AlphaStack - [#8448](#8448) [`590e495fd`](590e495) Thanks [@alex-page](https://github.com/alex-page)! - [Text] adds optional numeric font variant to render characters with a monospace apperance and consistent height and width - [#8130](#8130) [`6c0dda128`](6c0dda1) Thanks [@mrcthms](https://github.com/mrcthms)! - - Added a `suffix` prop to `Tooltip` - Improved the UX of `Tooltip` by refining open and close animations and adding an arrow pointing to the center of the `activator` - Added the `EmpemeralPresenceManager` to manage the presence of non-blocking overlays, like `Tooltip` and `Toast` - [#8556](#8556) [`de3a925a7`](de3a925) Thanks [@laurkim](https://github.com/laurkim)! - Deprecated `Stack` and `Stack.Item` and updated documentation on style guide - [#8498](#8498) [`624751155`](6247511) Thanks [@lgriffee](https://github.com/lgriffee)! - Migrated `legacy` custom properties from `v10` to `v11` ### Patch Changes - [#8534](#8534) [`eeb8a4fc5`](eeb8a4f) Thanks [@laurkim](https://github.com/laurkim)! - Migrated usage of `Stack` to `LegacyStack` - [#8540](#8540) [`70c166290`](70c1662) Thanks [@Ipriyankrajai](https://github.com/Ipriyankrajai)! - Fixed `ActionList` item `suffix` having extra padding when wrapped - [#8523](#8523) [`f644ad671`](f644ad6) Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed deprecation warning on Card - [#8531](#8531) [`7ecc8a9a6`](7ecc8a9) Thanks [@chloerice](https://github.com/chloerice)! - - Fixed header of `IndexTable` with `bulkActions` not having border radius - Fixed header of `selectable` `ResourceList` not having border radius - Fixed `selectable` `ResourceList` with no `bulkActions` having extra bottom padding in select mode - Fixed last `ResourceItem` in a `selectable` `ResourceList` with `bulkActions` having a border radius when in select mode - Updated dependencies \[[`6c0dda128`](6c0dda1), [`886da4fc3`](886da4f)]: - @shopify/polaris-tokens@6.7.0 - @shopify/polaris-icons@6.11.3 ## @shopify/polaris-tokens@6.7.0 ### Minor Changes - [#8130](#8130) [`6c0dda128`](6c0dda1) Thanks [@mrcthms](https://github.com/mrcthms)! - - Added a `suffix` prop to `Tooltip` - Improved the UX of `Tooltip` by refining open and close animations and adding an arrow pointing to the center of the `activator` - Added the `EmpemeralPresenceManager` to manage the presence of non-blocking overlays, like `Tooltip` and `Toast` ## @shopify/polaris-cli@0.1.14 ### Patch Changes - Updated dependencies \[[`e21fd8d79`](e21fd8d)]: - @shopify/polaris-migrator@0.15.0 ## @shopify/polaris-icons@6.11.3 ### Patch Changes - [#8505](#8505) [`886da4fc3`](886da4f) Thanks [@lfroms](https://github.com/lfroms)! - Converted percentage fill-opacity to float in two icons for better compatibility ## @shopify/stylelint-polaris@8.0.1 ### Patch Changes - Updated dependencies \[[`6c0dda128`](6c0dda1)]: - @shopify/polaris-tokens@6.7.0 ## polaris.shopify.com@0.38.0 ### Minor Changes - [#8129](#8129) [`6d3843755`](6d38437) Thanks [@jesstelford](https://github.com/jesstelford)! - Patterns documentation is now written in Markdown to match all other site content. - [#8130](#8130) [`6c0dda128`](6c0dda1) Thanks [@mrcthms](https://github.com/mrcthms)! - - Added a `suffix` prop to `Tooltip` - Improved the UX of `Tooltip` by refining open and close animations and adding an arrow pointing to the center of the `activator` - Added the `EmpemeralPresenceManager` to manage the presence of non-blocking overlays, like `Tooltip` and `Toast` - [#8559](#8559) [`668afa51c`](668afa5) Thanks [@jjgali](https://github.com/jjgali)! - Update voice and tone of Trustworthy experience value. ### Patch Changes - [#8534](#8534) [`eeb8a4fc5`](eeb8a4f) Thanks [@laurkim](https://github.com/laurkim)! - Migrated usage of `Stack` to `LegacyStack` - [#8519](#8519) [`f411cf0b6`](f411cf0) Thanks [@kyledurand](https://github.com/kyledurand)! - Added missing redirect, updated content - [#8553](#8553) [`d2672648a`](d267264) Thanks [@gwyneplaine](https://github.com/gwyneplaine)! - Fix legacy patterns wrongly being categorised as deprecated. These are now correctly labeled "legacy" - [#8556](#8556) [`de3a925a7`](de3a925) Thanks [@laurkim](https://github.com/laurkim)! - Deprecated `Stack` and `Stack.Item` and updated documentation on style guide - [#8493](#8493) [`a6fb3fbb7`](a6fb3fb) Thanks [@kyledurand](https://github.com/kyledurand)! - Moved props.json to cache and automate generation - [#8528](#8528) [`191ff96df`](191ff96) Thanks [@jesstelford](https://github.com/jesstelford)! - Fixed grid alignment on component group page - Updated dependencies \[[`eeb8a4fc5`](eeb8a4f), [`78d686db5`](78d686d), [`70c166290`](70c1662), [`b77c1fe51`](b77c1fe), [`bec50d4d8`](bec50d4), [`f644ad671`](f644ad6), [`590e495fd`](590e495), [`6c0dda128`](6c0dda1), [`de3a925a7`](de3a925), [`886da4fc3`](886da4f), [`7ecc8a9a6`](7ecc8a9), [`624751155`](6247511)]: - @shopify/polaris@10.33.0 - @shopify/polaris-tokens@6.7.0 - @shopify/polaris-icons@6.11.3 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…d titles and add best practice documentation for numeric values in tables (Shopify#8448) Co-authored-by: Rick Caplan <rick.caplan@shopify.com>
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @shopify/polaris-migrator@0.15.0 ### Minor Changes - [Shopify#8229](Shopify#8229) [`e21fd8d79`](Shopify@e21fd8d) Thanks [@aveline](https://github.com/aveline)! - Created migration to rename `Card` to `LegacyCard` ### Patch Changes - Updated dependencies \[[`6c0dda128`](Shopify@6c0dda1)]: - @shopify/polaris-tokens@6.7.0 - @shopify/stylelint-polaris@8.0.1 ## @shopify/polaris@10.33.0 ### Minor Changes - [Shopify#8514](Shopify#8514) [`78d686db5`](Shopify@78d686d) Thanks [@lgriffee](https://github.com/lgriffee)! - Add additional changes for migrating `legacy` custom properties from `v10` to `v11` - [Shopify#8512](Shopify#8512) [`b77c1fe51`](Shopify@b77c1fe) Thanks [@fatimasajadi](https://github.com/fatimasajadi)! - Add onMouseEnter prop to the each item on the ActionList component - [Shopify#8530](Shopify#8530) [`bec50d4d8`](Shopify@bec50d4) Thanks [@kyledurand](https://github.com/kyledurand)! - Removed alignment defaults from Inline and AlphaStack - [Shopify#8448](Shopify#8448) [`590e495fd`](Shopify@590e495) Thanks [@alex-page](https://github.com/alex-page)! - [Text] adds optional numeric font variant to render characters with a monospace apperance and consistent height and width - [Shopify#8130](Shopify#8130) [`6c0dda128`](Shopify@6c0dda1) Thanks [@mrcthms](https://github.com/mrcthms)! - - Added a `suffix` prop to `Tooltip` - Improved the UX of `Tooltip` by refining open and close animations and adding an arrow pointing to the center of the `activator` - Added the `EmpemeralPresenceManager` to manage the presence of non-blocking overlays, like `Tooltip` and `Toast` - [Shopify#8556](Shopify#8556) [`de3a925a7`](Shopify@de3a925) Thanks [@laurkim](https://github.com/laurkim)! - Deprecated `Stack` and `Stack.Item` and updated documentation on style guide - [Shopify#8498](Shopify#8498) [`624751155`](Shopify@6247511) Thanks [@lgriffee](https://github.com/lgriffee)! - Migrated `legacy` custom properties from `v10` to `v11` ### Patch Changes - [Shopify#8534](Shopify#8534) [`eeb8a4fc5`](Shopify@eeb8a4f) Thanks [@laurkim](https://github.com/laurkim)! - Migrated usage of `Stack` to `LegacyStack` - [Shopify#8540](Shopify#8540) [`70c166290`](Shopify@70c1662) Thanks [@Ipriyankrajai](https://github.com/Ipriyankrajai)! - Fixed `ActionList` item `suffix` having extra padding when wrapped - [Shopify#8523](Shopify#8523) [`f644ad671`](Shopify@f644ad6) Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed deprecation warning on Card - [Shopify#8531](Shopify#8531) [`7ecc8a9a6`](Shopify@7ecc8a9) Thanks [@chloerice](https://github.com/chloerice)! - - Fixed header of `IndexTable` with `bulkActions` not having border radius - Fixed header of `selectable` `ResourceList` not having border radius - Fixed `selectable` `ResourceList` with no `bulkActions` having extra bottom padding in select mode - Fixed last `ResourceItem` in a `selectable` `ResourceList` with `bulkActions` having a border radius when in select mode - Updated dependencies \[[`6c0dda128`](Shopify@6c0dda1), [`886da4fc3`](Shopify@886da4f)]: - @shopify/polaris-tokens@6.7.0 - @shopify/polaris-icons@6.11.3 ## @shopify/polaris-tokens@6.7.0 ### Minor Changes - [Shopify#8130](Shopify#8130) [`6c0dda128`](Shopify@6c0dda1) Thanks [@mrcthms](https://github.com/mrcthms)! - - Added a `suffix` prop to `Tooltip` - Improved the UX of `Tooltip` by refining open and close animations and adding an arrow pointing to the center of the `activator` - Added the `EmpemeralPresenceManager` to manage the presence of non-blocking overlays, like `Tooltip` and `Toast` ## @shopify/polaris-cli@0.1.14 ### Patch Changes - Updated dependencies \[[`e21fd8d79`](Shopify@e21fd8d)]: - @shopify/polaris-migrator@0.15.0 ## @shopify/polaris-icons@6.11.3 ### Patch Changes - [Shopify#8505](Shopify#8505) [`886da4fc3`](Shopify@886da4f) Thanks [@lfroms](https://github.com/lfroms)! - Converted percentage fill-opacity to float in two icons for better compatibility ## @shopify/stylelint-polaris@8.0.1 ### Patch Changes - Updated dependencies \[[`6c0dda128`](Shopify@6c0dda1)]: - @shopify/polaris-tokens@6.7.0 ## polaris.shopify.com@0.38.0 ### Minor Changes - [Shopify#8129](Shopify#8129) [`6d3843755`](Shopify@6d38437) Thanks [@jesstelford](https://github.com/jesstelford)! - Patterns documentation is now written in Markdown to match all other site content. - [Shopify#8130](Shopify#8130) [`6c0dda128`](Shopify@6c0dda1) Thanks [@mrcthms](https://github.com/mrcthms)! - - Added a `suffix` prop to `Tooltip` - Improved the UX of `Tooltip` by refining open and close animations and adding an arrow pointing to the center of the `activator` - Added the `EmpemeralPresenceManager` to manage the presence of non-blocking overlays, like `Tooltip` and `Toast` - [Shopify#8559](Shopify#8559) [`668afa51c`](Shopify@668afa5) Thanks [@jjgali](https://github.com/jjgali)! - Update voice and tone of Trustworthy experience value. ### Patch Changes - [Shopify#8534](Shopify#8534) [`eeb8a4fc5`](Shopify@eeb8a4f) Thanks [@laurkim](https://github.com/laurkim)! - Migrated usage of `Stack` to `LegacyStack` - [Shopify#8519](Shopify#8519) [`f411cf0b6`](Shopify@f411cf0) Thanks [@kyledurand](https://github.com/kyledurand)! - Added missing redirect, updated content - [Shopify#8553](Shopify#8553) [`d2672648a`](Shopify@d267264) Thanks [@gwyneplaine](https://github.com/gwyneplaine)! - Fix legacy patterns wrongly being categorised as deprecated. These are now correctly labeled "legacy" - [Shopify#8556](Shopify#8556) [`de3a925a7`](Shopify@de3a925) Thanks [@laurkim](https://github.com/laurkim)! - Deprecated `Stack` and `Stack.Item` and updated documentation on style guide - [Shopify#8493](Shopify#8493) [`a6fb3fbb7`](Shopify@a6fb3fb) Thanks [@kyledurand](https://github.com/kyledurand)! - Moved props.json to cache and automate generation - [Shopify#8528](Shopify#8528) [`191ff96df`](Shopify@191ff96) Thanks [@jesstelford](https://github.com/jesstelford)! - Fixed grid alignment on component group page - Updated dependencies \[[`eeb8a4fc5`](Shopify@eeb8a4f), [`78d686db5`](Shopify@78d686d), [`70c166290`](Shopify@70c1662), [`b77c1fe51`](Shopify@b77c1fe), [`bec50d4d8`](Shopify@bec50d4), [`f644ad671`](Shopify@f644ad6), [`590e495fd`](Shopify@590e495), [`6c0dda128`](Shopify@6c0dda1), [`de3a925a7`](Shopify@de3a925), [`886da4fc3`](Shopify@886da4f), [`7ecc8a9a6`](Shopify@7ecc8a9), [`624751155`](Shopify@6247511)]: - @shopify/polaris@10.33.0 - @shopify/polaris-tokens@6.7.0 - @shopify/polaris-icons@6.11.3 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
WHY are these changes introduced?
Numerical values should follow best practices found in spreadsheets which is to right align the values.
WHAT is this pull request doing?
IndexTable
examples to align numerical values to the rightIndexTables
Text
component