-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Use semantic Text
component for component typography
#11547
Conversation
624e55f
to
74dfd0d
Compare
Text
component98184ba
to
ba02545
Compare
Text
component for component typography
ba02545
to
a1770c5
Compare
e0841c4
to
8b62a6f
Compare
b7aa3e2
to
6414fd7
Compare
d11f424
to
ba50be9
Compare
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.
Amazing 💯 ⭐
A bit of feedback on some UI inconsistencies when reviewing chromatic but looks good overall!
polaris-react/src/components/ActionList/components/Item/Item.tsx
Outdated
Show resolved
Hide resolved
polaris-react/src/components/Listbox/components/TextOption/TextOption.tsx
Outdated
Show resolved
Hide resolved
polaris-react/src/components/Listbox/components/TextOption/TextOption.tsx
Outdated
Show resolved
Hide resolved
polaris-react/src/components/Navigation/components/Item/Item.tsx
Outdated
Show resolved
Hide resolved
polaris-react/src/components/Page/components/Header/components/Title/Title.tsx
Show resolved
Hide resolved
polaris-react/src/components/Filters/components/FiltersBar/FiltersBar.tsx
Outdated
Show resolved
Hide resolved
2d835b0
to
0e0805b
Compare
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.
Awesome work @sam-b-rose!! My review focused on tophatting the new Text
variants, Text
styles, @shopify/stylelint-polaris
rule changes, and Polaris site updates, LGTM 🚀
12da387
to
0e92866
Compare
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.
Some feedback from tophatting chromatic before most recent force push, I can double check the new chromatic again in a separate review
polaris-react/src/components/DropZone/components/FileUpload/FileUpload.module.css
Show resolved
Hide resolved
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.
Overall re-review looks good! 💯 Thanks for addressing all the earlier feedback 🤝
I've gone and accepted the chromatic diff changes for things we've discussed (i.e., DropZone button styles, Page header letter spacing, SearchField font size). I left any visual regressions I came across while tophatting or for things still pending discussion (i.e., Monchrome plain buttons).
polaris-react/src/components/ActionList/components/Item/Item.tsx
Outdated
Show resolved
Hide resolved
e337379
to
b3ad791
Compare
This fix improves text size alignment on the FilterBar component. The "Clear all" button is now the same size as the other buttons in the FilterBar.
b3ad791
to
1a05dc9
Compare
/snapit |
🫰✨ Thanks @sam-b-rose! Your snapshots have been published to npm. Test the snapshots by updating your yarn add @shopify/polaris-icons@0.0.0-snapshot-20240326142312 yarn add @shopify/polaris-migrator@0.0.0-snapshot-20240326142312 yarn add @shopify/polaris@0.0.0-snapshot-20240326142312 yarn add @shopify/polaris-tokens@0.0.0-snapshot-20240326142312 yarn add @shopify/stylelint-polaris@0.0.0-snapshot-20240326142312 |
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@12.24.0 ### Minor Changes - [#11547](#11547) [`df5276317`](df52763) Thanks [@sam-b-rose](https://github.com/sam-b-rose)! - Applied semantic type styles using the `Text` component - [#11728](#11728) [`281c8f8e9`](281c8f8) Thanks [@kyledurand](https://github.com/kyledurand)! - Added new AlphaPicker component - [#11645](#11645) [`b726dadbb`](b726dad) Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Added `useCopyToClipboard` hook for building copy actions matching common actions guidelines - [#11780](#11780) [`4fffc2dcc`](4fffc2d) Thanks [@itwasmattgregg](https://github.com/itwasmattgregg)! - allows icons to be displayed on primary actions on Page component - [#11547](#11547) [`df5276317`](df52763) Thanks [@sam-b-rose](https://github.com/sam-b-rose)! - Added `base`,`inherit`, `disabled`, and `text-inverse` tone options for Text component - [#11547](#11547) [`df5276317`](df52763) Thanks [@sam-b-rose](https://github.com/sam-b-rose)! - Updated plain/monochrome Button text size to bodySm for micro ### Patch Changes - [#11789](#11789) [`36df1aa6c`](36df1aa) Thanks [@laurkim](https://github.com/laurkim)! - Fixed logo spacing on `ContextualSaveBar` - [#11794](#11794) [`ffdcf1df7`](ffdcf1d) Thanks [@kyledurand](https://github.com/kyledurand)! - Set default scrollbar width to thin on scrollable - [#11804](#11804) [`d1b46c25c`](d1b46c2) Thanks [@laurkim](https://github.com/laurkim)! - Fixed layout shift on `EmptyState` when image is loading with skeleton image ## @shopify/stylelint-polaris@15.5.0 ### Minor Changes - [#11547](#11547) [`df5276317`](df52763) Thanks [@sam-b-rose](https://github.com/sam-b-rose)! - Added warning for `font-size`, `line-height`, and `font-weight` properties. Use the `Text` component as a preferred option. ## @shopify/polaris-migrator@0.28.4 ### Patch Changes - Updated dependencies \[[`df5276317`](df52763)]: - @shopify/stylelint-polaris@15.5.0 ## polaris.shopify.com@0.67.0 ### Minor Changes - [#11779](#11779) [`86a6ba44a`](86a6ba4) Thanks [@itwasmattgregg](https://github.com/itwasmattgregg)! - Added examples for `Card` and `Page` with icon actions ### Patch Changes - [#11547](#11547) [`df5276317`](df52763) Thanks [@sam-b-rose](https://github.com/sam-b-rose)! - Added page for `typography/property-disallow-list` Stylelint rule - Updated dependencies \[[`df5276317`](df52763), [`281c8f8e9`](281c8f8), [`b726dadbb`](b726dad), [`4fffc2dcc`](4fffc2d), [`df5276317`](df52763), [`36df1aa6c`](36df1aa), [`df5276317`](df52763), [`ffdcf1df7`](ffdcf1d), [`d1b46c25c`](d1b46c2)]: - @shopify/polaris@12.24.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Closes https://github.com/Shopify/polaris-internal/issues/1483 This PR replaces seeks to reduce the need to semantic CSS custom properties for typography by: - Replacing font styles with the semantic `Text` equivalent - Remove typography overrides (i.e., `LegacyCard`) - Remove semantic typography tokens from `Text` - **Why?** Using the semantic tokens doesn't support responsive typography The goal is to allow the `Text` component to apply semantic type styles rather using CSS custom properties. Using the theme and CSS tokens should be a last resort for styling. Using components to compose styles reduces the surface area of maintenance and the likelihood of implementation bugs. ### WHAT is this pull request doing? - Adds `base`, `inherit`, and `text-inverse-secondary` tones to `Text` component - Uses the `Text` component to apply typography styles for Polaris components where possible - Updates the `@shopify/stylelint-polaris` rule to include the following as a warning in the property disallow list: - `font-size` - `font-weight` - `line-height` - `letter-spacing` - Adds a page on polaris.shopify.com for the `typography/property-disallow-list` rule #### Components updated - `BulkActions` - `CheckableButton` - `DropZone` - `FooterHelp` - `Frame` - `Toast` - `InlineError` - `IndexFilters` - `IndexTable` - `Label` - `MediaCard` - `LegacyCard` - `Navigation` - `Page` - `Select` - `SelectAllActoins` - `SkeletonPage` - `Tabs` - `Text` (added `base`, `inherit`, and `text-inverse-secondary` tone) - `TextField` - `TopBar/UserMenu` #### Stylelint rule in-action https://github.com/Shopify/polaris/assets/11774595/d60c3c77-022d-4799-b2ea-5309a8b96bbb #### New docs page for Stylelint rule <img width="822" alt="Screenshot 2024-03-21 at 3 53 01 PM" src="https://github.com/Shopify/polaris/assets/11774595/6d28281d-0efc-4961-8319-aadd6b5967e5"> ### How to 🎩 🖥 [Local development instructions](https://github.com/Shopify/polaris/blob/main/README.md#install-dependencies-and-build-workspaces) 🗒 [General tophatting guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md) 📄 [Changelog guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog) ### 🎩 checklist - [x] Tested a [snapshot](https://github.com/Shopify/polaris/blob/main/documentation/Releasing.md#-snapshot-releases) - [x] Tested on [mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing) - [ ] Tested on [multiple browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers) - [x] Tested for [accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md) - [ ] Updated the component's `README.md` with documentation changes - [ ] [Tophatted documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md) changes in the style guide
Closes https://github.com/Shopify/polaris-internal/issues/1483
This PR replaces seeks to reduce the need to semantic CSS custom properties for typography by:
Text
equivalentLegacyCard
)Text
The goal is to allow the
Text
component to apply semantic type styles rather using CSS custom properties. Using the theme and CSS tokens should be a last resort for styling. Using components to compose styles reduces the surface area of maintenance and the likelihood of implementation bugs.WHAT is this pull request doing?
base
,inherit
, andtext-inverse-secondary
tones toText
componentText
component to apply typography styles for Polaris components where possible@shopify/stylelint-polaris
rule to include the following as a warning in the property disallow list:font-size
font-weight
line-height
letter-spacing
typography/property-disallow-list
ruleComponents updated
BulkActions
CheckableButton
DropZone
FooterHelp
Frame
Toast
InlineError
IndexFilters
IndexTable
Label
MediaCard
LegacyCard
Navigation
Page
Select
SelectAllActoins
SkeletonPage
Tabs
Text
(addedbase
,inherit
, andtext-inverse-secondary
tone)TextField
TopBar/UserMenu
Stylelint rule in-action
Screen.Recording.2024-03-21.at.3.02.40.PM.mov
New docs page for Stylelint rule
How to 🎩
🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines
🎩 checklist
README.md
with documentation changes