From 15348ff5a94d84740d362237cc685a08d81c53bd Mon Sep 17 00:00:00 2001 From: beefchimi Date: Tue, 13 Oct 2020 15:00:38 -0400 Subject: [PATCH 01/11] :art: [Button] Organize some props --- src/components/Button/Button.tsx | 83 ++++++++++++------- .../UnstyledButton/UnstyledButton.tsx | 74 ++++++++--------- src/utilities/focus.ts | 8 +- 3 files changed, 88 insertions(+), 77 deletions(-) diff --git a/src/components/Button/Button.tsx b/src/components/Button/Button.tsx index 4331cf99d39..95557bb850d 100644 --- a/src/components/Button/Button.tsx +++ b/src/components/Button/Button.tsx @@ -2,7 +2,10 @@ import React, {useRef, useState, useCallback} from 'react'; import {CaretDownMinor} from '@shopify/polaris-icons'; import {classNames, variationName} from '../../utilities/css'; -import {handleMouseUpByBlurring} from '../../utilities/focus'; +import { + handleMouseUpByBlurring, + MouseUpBlurHandler, +} from '../../utilities/focus'; import {useFeatures} from '../../utilities/features'; import {useI18n} from '../../utilities/i18n'; import {Icon} from '../Icon'; @@ -91,14 +94,37 @@ export interface ButtonProps { onTouchStart?(): void; } +interface CommonButtonProps { + id: ButtonProps['id']; + className: string; +} + +interface InteractiveButtonProps + extends CommonButtonProps, + Pick< + ButtonProps, + | 'accessibilityLabel' + | 'onClick' + | 'onFocus' + | 'onBlur' + | 'onMouseEnter' + | 'onTouchStart' + > { + onMouseUp: MouseUpBlurHandler; +} + const DEFAULT_SIZE = 'medium'; export function Button({ id, + children, url, disabled, + external, + download, + submit, loading, - children, + pressed, accessibilityLabel, ariaControls, ariaExpanded, @@ -111,8 +137,6 @@ export function Button({ onKeyUp, onMouseEnter, onTouchStart, - external, - download, icon, primary, outline, @@ -120,11 +144,9 @@ export function Button({ disclosure, plain, monochrome, - submit, size = DEFAULT_SIZE, textAlign, fullWidth, - pressed, connectedDisclosure, }: ButtonProps) { const {newDesignLanguage} = useFeatures(); @@ -226,7 +248,6 @@ export function Button({ ); - const type = submit ? 'submit' : 'button'; const ariaPressedStatus = pressed !== undefined ? pressed : ariaPressed; const [disclosureActive, setDisclosureActive] = useState(false); @@ -291,28 +312,34 @@ export function Button({ let buttonMarkup; + const commonProps: CommonButtonProps = { + id, + className, + }; + const interactiveProps: InteractiveButtonProps = { + ...commonProps, + accessibilityLabel, + onClick, + onFocus, + onBlur, + onMouseUp: handleMouseUpByBlurring, + onMouseEnter, + onTouchStart, + }; + if (url) { buttonMarkup = isDisabled ? ( // Render an `` so toggling disabled/enabled state changes only the // `href` attribute instead of replacing the whole element. - // eslint-disable-next-line jsx-a11y/anchor-is-valid - + {content} ) : ( {content} @@ -320,23 +347,15 @@ export function Button({ } else { buttonMarkup = ( diff --git a/src/components/UnstyledButton/UnstyledButton.tsx b/src/components/UnstyledButton/UnstyledButton.tsx index 065378792de..59f81307044 100644 --- a/src/components/UnstyledButton/UnstyledButton.tsx +++ b/src/components/UnstyledButton/UnstyledButton.tsx @@ -6,20 +6,20 @@ import {UnstyledLink} from '../UnstyledLink'; export interface UnstyledButtonProps { /** The content to display inside the button */ children?: React.ReactNode; - /** A destination to link to, rendered in the href attribute of a link */ - url?: string; /** A unique identifier for the button */ id?: string; /** A custom class name to apply styles to button */ className?: string; - /** Disables the button, disallowing merchant interaction */ - disabled?: boolean; - /** Allows the button to submit a form */ - submit?: boolean; + /** A destination to link to, rendered in the href attribute of a link */ + url?: string; /** Forces url to open in a new tab */ external?: boolean; /** Tells the browser to download the url instead of opening it. Provides a hint for the downloaded filename if it is a string value */ download?: string | boolean; + /** Allows the button to submit a form */ + submit?: boolean; + /** Disables the button, disallowing merchant interaction */ + disabled?: boolean; /** Sets the button in a pressed state */ pressed?: boolean; /** Visually hidden text for screen readers */ @@ -54,10 +54,13 @@ export interface UnstyledButtonProps { export function UnstyledButton({ id, - url, - disabled, children, className, + url, + external, + download, + submit, + disabled, pressed, accessibilityLabel, ariaControls, @@ -71,9 +74,6 @@ export function UnstyledButton({ onKeyUp, onMouseEnter, onTouchStart, - external, - download, - submit, ...rest }: UnstyledButtonProps) { const hasGivenDeprecationWarning = useRef(false); @@ -86,38 +86,38 @@ export function UnstyledButton({ hasGivenDeprecationWarning.current = true; } - const type = submit ? 'submit' : 'button'; const ariaPressedStatus = pressed !== undefined ? pressed : ariaPressed; let buttonMarkup; + const commonProps = { + id, + className, + 'aria-label': accessibilityLabel, + }; + const interactiveProps = { + ...commonProps, + onClick, + onFocus, + onBlur, + onMouseUp: handleMouseUpByBlurring, + onMouseEnter, + onTouchStart, + }; + if (url) { buttonMarkup = disabled ? ( // Render an `` so toggling disabled/enabled state changes only the // `href` attribute instead of replacing the whole element. - // eslint-disable-next-line jsx-a11y/anchor-is-valid - + {children} ) : ( @@ -127,23 +127,15 @@ export function UnstyledButton({ } else { buttonMarkup = ( ); + expect(button.find(UnstyledButton).text()).toContain(mockChildren); + }); + }); + + describe('id', () => { + it('passes prop', () => { + const id = 'MockId'; + const button = mountWithAppProvider(); - expect(button.text()).toContain(label); + describe('download', () => { + const mockUrl = 'https://google.com'; + + it('gets passed into the link as a boolean', () => { + const button = mountWithAppProvider(, + , ); expect(button.find(Spinner).exists()).toBeTruthy(); }); - it('sets an alert role on the button', () => { - const button = mountWithAppProvider(, - ).find('button'); + ).find(UnstyledButton); trigger(button, 'onKeyPress'); expect(spy).toHaveBeenCalled(); @@ -442,7 +456,7 @@ describe(', - ).find('button'); + ).find(UnstyledButton); trigger(button, 'onKeyUp'); expect(spy).toHaveBeenCalled(); }); @@ -453,7 +467,7 @@ describe(', - ).find('button'); + ).find(UnstyledButton); trigger(button, 'onKeyDown'); expect(spy).toHaveBeenCalled(); }); @@ -464,21 +478,21 @@ describe('); expect(button.find(UnstyledButton).text()).toContain(mockChildren); }); @@ -27,76 +26,34 @@ describe('