Skip to content

Commit

Permalink
Use semantic Text component for component typography (#11547)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sam-b-rose committed Mar 27, 2024
1 parent 86a6ba4 commit df52763
Show file tree
Hide file tree
Showing 106 changed files with 1,388 additions and 1,115 deletions.
5 changes: 5 additions & 0 deletions .changeset/cool-dolls-lie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': minor
---

Applied semantic type styles using the `Text` component
5 changes: 5 additions & 0 deletions .changeset/lazy-flies-relate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': minor
---

Added `base`,`inherit`, `disabled`, and `text-inverse` tone options for Text component
5 changes: 5 additions & 0 deletions .changeset/long-days-wonder.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'polaris.shopify.com': patch
---

Added page for `typography/property-disallow-list` Stylelint rule
5 changes: 5 additions & 0 deletions .changeset/nine-cameras-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/stylelint-polaris': minor
---

Added warning for `font-size`, `line-height`, and `font-weight` properties. Use the `Text` component as a preferred option.
5 changes: 5 additions & 0 deletions .changeset/plenty-shoes-remain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': minor
---

Updated plain/monochrome Button text size to bodySm for micro
3 changes: 2 additions & 1 deletion polaris-react/postcss-mixins/text-style-input.css
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
letter-spacing: initial;

@media (--p-breakpoints-md-up) {
font-size: var(--p-font-size-350);
font-size: var(--p-font-size-325);
line-height: var(--p-font-line-height-500);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ export function Default() {
const details = connected ? 'Account connected' : 'No account connected';

const terms = connected ? null : (
<p>
<Text as="p" variant="bodyMd">
By clicking Connect, you agree to accept Sample App’s{' '}
<Link url="Example App">terms and conditions</Link>. You’ll pay a
commission rate of 15% on sales made through Sample App.
</p>
</Text>
);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,16 @@ export function AccountConnection({
);

const detailsMarkup = details ? (
<Text as="span" tone="subdued">
<Text as="span" variant="bodyMd" tone="subdued">
{details}
</Text>
) : null;

const termsOfServiceMarkup = termsOfService ? (
<Box paddingBlockStart={breakpoints.mdUp ? '400' : '500'}>
{termsOfService}
<Text as="span" variant="bodyMd">
{termsOfService}
</Text>
</Box>
) : null;

Expand Down
20 changes: 18 additions & 2 deletions polaris-react/src/components/ActionList/components/Item/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,13 @@ export function Item({
</Text>
</>
) : (
contentText
<Text
as="span"
variant="bodyMd"
fontWeight={active ? 'semibold' : 'regular'}
>
{contentText}
</Text>
);

const badgeMarkup = badge && (
Expand All @@ -103,7 +109,17 @@ export function Item({
</Box>
);

const textMarkup = <span className={styles.Text}>{contentMarkup}</span>;
const textMarkup = (
<span className={styles.Text}>
<Text
as="span"
variant="bodyMd"
fontWeight={active ? 'semibold' : 'regular'}
>
{contentMarkup}
</Text>
</span>
);

const contentElement = (
<InlineStack blockAlign="center" gap="150" wrap={false}>
Expand Down
28 changes: 15 additions & 13 deletions polaris-react/src/components/AppProvider/AppProvider.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ export function Default(_, context) {

return (
<ResourceList.Item id={id} url={url} media={media}>
<h3>
<Text fontWeight="bold" as="span">
{name}
</Text>
</h3>
<div>{location}</div>
<Text as="h3" variant="bodyMd" fontWeight="bold">
{name}
</Text>
<Text as="p" variant="bodyMd">
{location}
</Text>
</ResourceList.Item>
);
}}
Expand Down Expand Up @@ -119,12 +119,12 @@ export function WithI18n() {

return (
<ResourceList.Item id={id} url={url} media={media}>
<h3>
<Text fontWeight="bold" as="span">
{name}
</Text>
</h3>
<div>{location}</div>
<Text as="h3" fontWeight="bold" variant="bodyMd">
{name}
</Text>
<Text as="p" variant="bodyMd">
{location}
</Text>
</ResourceList.Item>
);
}}
Expand Down Expand Up @@ -156,7 +156,9 @@ export function WithLinkComponent(_, context) {
title="Jar With Lock-Lid"
primaryAction={{content: 'Save', disabled: true}}
>
<p>Page content</p>
<Text as="p" variant="bodyMd">
Page content
</Text>
</Page>
</AppProvider>
);
Expand Down
9 changes: 7 additions & 2 deletions polaris-react/src/components/AppProvider/global.css
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,17 @@

html,
body {
font-size: var(--p-font-size-325);
line-height: var(--p-font-line-height-500);
font-size: var(--p-font-size-400);
line-height: var(--p-font-line-height-600);
font-weight: var(--p-font-weight-regular);
font-feature-settings: 'calt' 0;
letter-spacing: initial;
color: var(--p-color-text);

@media (--p-breakpoints-md-up) {
font-size: var(--p-font-size-325);
line-height: var(--p-font-line-height-500);
}
}

html,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,4 @@
min-width: 0;
max-width: 100%;
flex: 1 1 auto;

.ContentWrap {
/* stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY */
@mixin text-breakword;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ export function MappedAction({

let prefixMarkup: React.ReactNode | null = null;

const contentOverflowStyle = wrapOverflow ? styles.ContentWrap : undefined;

if (prefix) {
prefixMarkup = <div className={styles.Prefix}>{prefix}</div>;
} else if (icon) {
Expand Down Expand Up @@ -74,9 +72,11 @@ export function MappedAction({

const contentMarkup = (
<div className={styles.Text}>
<div className={contentOverflowStyle}>{contentText}</div>
<Text as="p" variant="bodyMd" breakWord={wrapOverflow}>
{contentText}
</Text>
{helpText ? (
<Text tone="subdued" as="span">
<Text as="p" variant="bodyMd" tone="subdued">
{helpText}
</Text>
) : null}
Expand Down
44 changes: 26 additions & 18 deletions polaris-react/src/components/Banner/Banner.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,20 @@ export default {
export function Default() {
return (
<Banner title="Order archived" onDismiss={() => {}}>
<p>This order was archived on March 7, 2017 at 3:12pm EDT.</p>
<Text as="p" variant="bodyMd">
This order was archived on March 7, 2017 at 3:12pm EDT.
</Text>
</Banner>
);
}

export function Dismissible() {
return (
<Banner onDismiss={() => {}}>
<p>
<Text as="p" variant="bodyMd">
Use your finance report to get detailed information about your business.{' '}
<Link url="">Let us know what you think</Link>
</p>
</Text>
</Banner>
);
}
Expand All @@ -48,10 +50,10 @@ export function WithFooterCallToAction() {
secondaryAction={{content: 'Learn more', url: ''}}
onDismiss={() => {}}
>
<p>
<Text as="p" variant="bodyMd">
Add weights to show accurate rates at checkout and when buying shipping
labels in Shopify.
</p>
</Text>
</Banner>
);
}
Expand All @@ -65,7 +67,9 @@ export function Informational() {
tone="info"
onDismiss={() => {}}
>
<p>Make sure you know how these changes affect your store.</p>
<Text as="p" variant="bodyMd">
Make sure you know how these changes affect your store.
</Text>
</Banner>
);
}
Expand Down Expand Up @@ -105,11 +109,11 @@ export function Critical() {
action={{content: 'Review risk analysis'}}
tone="critical"
>
<p>
<Text as="p" variant="bodyMd">
Before fulfilling this order or capturing payment, please{' '}
<Link url="">review the Risk Analysis</Link> and determine if this order
is fraudulent.
</p>
</Text>
</Banner>
);
}
Expand Down Expand Up @@ -140,15 +144,15 @@ export function InAModal() {
<Modal.Section>
<TextContainer>
<Banner action={{content: 'Connect account'}} tone="warning">
<p>
<Text as="p" variant="bodyMd">
Connect your instagram account to your shop before proceeding.
</p>
</Text>
</Banner>
<p>
<Text as="p" variant="bodyMd">
Use Instagram posts to share your products with millions of
people. Let shoppers buy from your store without leaving
Instagram.
</p>
</Text>
</TextContainer>
</Modal.Section>
</Modal>
Expand All @@ -168,10 +172,10 @@ export function WithFocus() {
tone="critical"
ref={banner}
>
<p>
<Text as="p" variant="bodyMd">
Before fulfilling this order or capturing payment, please review the
fraud analysis and determine if this order is fraudulent
</p>
</Text>
</Banner>
);
}
Expand Down Expand Up @@ -200,13 +204,15 @@ export function InALegacyCard() {
<LegacyCard title="Online store dashboard" sectioned>
<TextContainer>
<Banner onDismiss={() => {}}>
<p>
<Text as="p" variant="bodyMd">
Use your finance report to get detailed information about your
business. <Link url="">Let us know what you think</Link>
</p>
</Text>
</Banner>

<p>View a summary of your online store’s performance.</p>
<Text as="p" variant="bodyMd">
View a summary of your online store’s performance.
</Text>
</TextContainer>
</LegacyCard>
);
Expand All @@ -224,7 +230,9 @@ export function WithEndJustifiedContent() {
Logs
</Link>
</InlineStack>
<p>This order was archived on March 7, 2017 at 3:12pm EDT.</p>
<Text as="p" variant="bodyMd">
This order was archived on March 7, 2017 at 3:12pm EDT.
</Text>
</BlockStack>
</Banner>
);
Expand Down
14 changes: 11 additions & 3 deletions polaris-react/src/components/Banner/Banner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -164,21 +164,29 @@ export function BannerLayout({
) : null,
};

const childrenMarkup = children ? (
<Text as="span" variant="bodyMd">
{children}
</Text>
) : null;

if (withinContentContainer) {
return (
<WithinContentContainerBanner {...sharedBannerProps}>
{children}
{childrenMarkup}
</WithinContentContainerBanner>
);
}

if (isInlineIconBanner) {
return (
<InlineIconBanner {...sharedBannerProps}>{children}</InlineIconBanner>
<InlineIconBanner {...sharedBannerProps}>
{childrenMarkup}
</InlineIconBanner>
);
}

return <DefaultBanner {...sharedBannerProps}>{children}</DefaultBanner>;
return <DefaultBanner {...sharedBannerProps}>{childrenMarkup}</DefaultBanner>;
}

export function DefaultBanner({
Expand Down
Loading

0 comments on commit df52763

Please sign in to comment.