From a30cc57bcea1ae7ba5395d42c0c91cee91516c98 Mon Sep 17 00:00:00 2001 From: Sam Rose <11774595+sam-b-rose@users.noreply.github.com> Date: Fri, 15 Mar 2024 13:51:24 -0400 Subject: [PATCH] Use the `Text` component to apply Button text styles (#11735) ### WHY are these changes introduced? Cleans up the button CSS by re-using the Text component styles to apply the proper `Button` typography. This PR also address some test failures: - `Button.test.tsx`: The `matchMedia` mock needed to be cleared - `TrapFocus.test.tsx`: The `Button` external element was being rendered outside of the test, avoiding the global `beforeEach`/`afterEach` test setup which mocks `matchMedia` needed by `Button`. > [!NOTE] > Depends on https://github.com/Shopify/polaris/pull/11724 Related to https://github.com/Shopify/polaris-internal/issues/1483 --- .changeset/ten-squids-nail.md | 5 +++ .../src/components/Button/Button.module.css | 17 +------- .../src/components/Button/Button.tsx | 20 +++++++-- .../components/Button/tests/Button.test.tsx | 41 ++++++++++++++----- .../TrapFocus/tests/TrapFocus.test.tsx | 9 +--- 5 files changed, 56 insertions(+), 36 deletions(-) create mode 100644 .changeset/ten-squids-nail.md diff --git a/.changeset/ten-squids-nail.md b/.changeset/ten-squids-nail.md new file mode 100644 index 00000000000..0f9d7b13953 --- /dev/null +++ b/.changeset/ten-squids-nail.md @@ -0,0 +1,5 @@ +--- +'@shopify/polaris': patch +--- + +Used `Text` component to apply text styles for `Button` diff --git a/polaris-react/src/components/Button/Button.module.css b/polaris-react/src/components/Button/Button.module.css index d701417f25f..ba01a680a65 100644 --- a/polaris-react/src/components/Button/Button.module.css +++ b/polaris-react/src/components/Button/Button.module.css @@ -34,19 +34,9 @@ border-radius: var(--p-border-radius-200); box-shadow: var(--pc-button-box-shadow); color: var(--pc-button-color); - - font-size: var(--p-font-size-325); - font-weight: var(--p-font-weight-medium); - line-height: var(--p-font-line-height-500); - cursor: pointer; user-select: none; -webkit-tap-highlight-color: transparent; - - @media (--p-breakpoints-md-up) { - font-size: var(--p-font-size-300); - line-height: var(--p-font-line-height-400); - } } /* https://developer.mozilla.org/en-US/docs/Web/CSS/Specificity#increasing_specificity_by_duplicating_selector @@ -166,7 +156,7 @@ --pc-button-color_active: var(--p-color-text-link-active); } -.variantPlain:is(:hover, :active, :focus-visible) { +.variantPlain:is(:hover, :active, :focus-visible):not(.removeUnderline) { text-decoration: underline; } @@ -179,9 +169,6 @@ --pc-button-bg_disabled: transparent; margin: calc(-1 * var(--pc-button-padding-block)) calc(-1 * var(--pc-button-padding-inline)); - font-size: var(--p-font-size-325); - font-weight: var(--p-font-weight-regular); - line-height: var(--p-font-line-height-400); } .variantPlain:focus-visible, @@ -252,8 +239,6 @@ .sizeLarge { --pc-button-padding-block: var(--p-space-150); --pc-button-padding-inline: var(--p-space-300); - font-size: var(--p-font-size-325); - line-height: var(--p-font-line-height-500); min-height: var(--p-height-900); min-width: var(--p-height-900); diff --git a/polaris-react/src/components/Button/Button.tsx b/polaris-react/src/components/Button/Button.tsx index 285fd2bee5d..57dab4772e4 100644 --- a/polaris-react/src/components/Button/Button.tsx +++ b/polaris-react/src/components/Button/Button.tsx @@ -5,6 +5,7 @@ import { ChevronUpIcon, } from '@shopify/polaris-icons'; +import {useBreakpoints} from '../../utilities/breakpoints'; import type {BaseButton, IconSource} from '../../types'; import {classNames, variationName} from '../../utilities/css'; import {handleMouseUpByBlurring} from '../../utilities/focus'; @@ -12,6 +13,8 @@ import type {MouseUpBlurHandler} from '../../utilities/focus'; import {useI18n} from '../../utilities/i18n'; import {Icon} from '../Icon'; import {Spinner} from '../Spinner'; +import {Text} from '../Text'; +import type {TextProps} from '../Text'; import {UnstyledButton} from '../UnstyledButton'; import type {UnstyledButtonProps} from '../UnstyledButton'; @@ -121,6 +124,7 @@ export function Button({ }: ButtonProps) { const i18n = useI18n(); const isDisabled = disabled || loading; + const {mdUp} = useBreakpoints(); const className = classNames( styles.Button, @@ -164,14 +168,24 @@ export function Button({ {iconSource} ) : null; + const hasPlainText = ['plain', 'monochromePlain'].includes(variant); + let textFontWeight: TextProps['fontWeight'] = 'medium'; + if (hasPlainText) { + textFontWeight = 'regular'; + } else if (variant === 'primary') { + textFontWeight = mdUp ? 'medium' : 'semibold'; + } + const childMarkup = children ? ( - {children} - + ) : null; const spinnerSVGMarkup = loading ? ( diff --git a/polaris-react/src/components/Button/tests/Button.test.tsx b/polaris-react/src/components/Button/tests/Button.test.tsx index 38743eb64ef..e587461e752 100644 --- a/polaris-react/src/components/Button/tests/Button.test.tsx +++ b/polaris-react/src/components/Button/tests/Button.test.tsx @@ -9,17 +9,27 @@ import {mountWithApp} from 'tests/utilities'; import {Icon} from '../../Icon'; import {Spinner} from '../../Spinner'; +import {Text} from '../../Text'; import {UnstyledButton} from '../../UnstyledButton'; import {Button} from '../Button'; +jest.mock('../../../utilities/breakpoints', () => ({ + ...(jest.requireActual('../../../utilities/breakpoints') as any), + useBreakpoints: jest.fn(), +})); + describe('); - const childrenSpan = button.find('span', {children})!; - expect(childrenSpan).toHaveReactProps({ - className: expect.stringContaining('removeUnderline'), - }); - }); }); describe('dataPrimaryLink', () => { @@ -390,4 +391,24 @@ describe('); + expect(wrapper).toContainReactComponent(Text, { + variant: 'bodySm', + fontWeight: 'medium', + children: 'Test', + }); + }); }); + +function mockUseBreakpoints(mdUp: boolean) { + const useBreakpoints: jest.Mock = jest.requireMock( + '../../../utilities/breakpoints', + ).useBreakpoints; + + useBreakpoints.mockReturnValue({ + mdUp, + }); +} diff --git a/polaris-react/src/components/TrapFocus/tests/TrapFocus.test.tsx b/polaris-react/src/components/TrapFocus/tests/TrapFocus.test.tsx index 6956ecec230..3a1978c2475 100644 --- a/polaris-react/src/components/TrapFocus/tests/TrapFocus.test.tsx +++ b/polaris-react/src/components/TrapFocus/tests/TrapFocus.test.tsx @@ -158,14 +158,8 @@ describe('', () => { }); describe('handleBlur', () => { - const externalDomNode = mountWithApp(