From dcf878bf299bcb7b6c4a5c9810109ff1b8a7f2c3 Mon Sep 17 00:00:00 2001 From: Daniel Leroux Date: Tue, 26 Nov 2019 12:40:39 -0500 Subject: [PATCH] update bulkactions --- UNRELEASED.md | 1 + src/components/ResourceItem/ResourceItem.scss | 2 +- src/components/ResourceList/ResourceList.tsx | 2 +- .../components/BulkActions/BulkActions.scss | 95 +++---------------- .../components/BulkActions/BulkActions.tsx | 62 ++++++------ .../BulkActionButton/BulkActionButton.tsx | 59 +++--------- .../BulkActions/tests/BulkActions.test.tsx | 18 ++++ .../CheckableButton/CheckableButton.scss | 50 ++++++---- .../CheckableButton/CheckableButton.tsx | 9 +- .../tests/CheckableButton.test.tsx | 27 ++++++ .../ResourceList/tests/ResourceList.test.tsx | 4 +- tests/build.test.js | 2 +- 12 files changed, 151 insertions(+), 180 deletions(-) diff --git a/UNRELEASED.md b/UNRELEASED.md index 4bc728a4faf..9944d410b63 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -45,5 +45,6 @@ - Changed `aria-labelledby` to always exist on `TextField` ([#2401](https://github.com/Shopify/polaris-react/pull/2401)) - Converted `ButtonGroup > Item` into a functional component ([#2441](https://github.com/Shopify/polaris-react/pull/2441)) +- Refactored BulkActions to make use of `ButtonGroup` ([#2441](https://github.com/Shopify/polaris-react/pull/2441)) ### Deprecations diff --git a/src/components/ResourceItem/ResourceItem.scss b/src/components/ResourceItem/ResourceItem.scss index 7017efa3c11..263d067b61a 100644 --- a/src/components/ResourceItem/ResourceItem.scss +++ b/src/components/ResourceItem/ResourceItem.scss @@ -187,7 +187,7 @@ $resource-list-item-variables: ( (-1 * resource-list-item(padding)) resource-list-item(control-indent); display: flex; - @include breakpoint-before(resource-list-item(breakpoint-small)) { + @include breakpoint-before(resource-list-item(breakpoint-small), false) { visibility: hidden; .selectMode & { diff --git a/src/components/ResourceList/ResourceList.tsx b/src/components/ResourceList/ResourceList.tsx index 6613e5e328f..d643c4be698 100644 --- a/src/components/ResourceList/ResourceList.tsx +++ b/src/components/ResourceList/ResourceList.tsx @@ -806,7 +806,7 @@ function defaultIdForItem(item: any, index: number) { function isSmallScreen() { return typeof window === 'undefined' ? false - : window.innerWidth <= SMALL_SCREEN_WIDTH; + : window.innerWidth < SMALL_SCREEN_WIDTH; } // Use named export once withAppProvider is refactored away diff --git a/src/components/ResourceList/components/BulkActions/BulkActions.scss b/src/components/ResourceList/components/BulkActions/BulkActions.scss index eec9e14e98d..b274b35a198 100644 --- a/src/components/ResourceList/components/BulkActions/BulkActions.scss +++ b/src/components/ResourceList/components/BulkActions/BulkActions.scss @@ -6,23 +6,6 @@ $bulk-actions-button-stacking-order: ( ); $bulk-actions-offset-slide-in-start: rem(-40px); -.Button { - @include text-style-button; - @include button-base; - - &:not(:first-child) { - margin-left: -1px; - } - - .Group-measuring & { - font-size: font-size(button); - } - - .disabled & { - @include base-button-disabled; - } -} - .Group { @include text-style-input; width: 100%; @@ -79,14 +62,17 @@ $bulk-actions-offset-slide-in-start: rem(-40px); } } -.ButtonGroup { - display: flex; - align-items: center; +.ButtonGroupWrapper { width: 100%; - flex-wrap: nowrap; - justify-content: flex-end; - box-shadow: inset 0 0 0 1px color('sky', 'dark'); - border-radius: border-radius(); + max-width: 100%; + + // We need the first item of button group on small screen to fill the space + @include breakpoint-before(resource-list(breakpoint-small)) { + // stylelint-disable-next-line selector-max-specificity, selector-max-class, selector-max-combinators, selector-max-type + > div > div:first-child { + flex: 1 1 auto; + } + } @include breakpoint-after(resource-list(breakpoint-small)) { width: auto; @@ -98,71 +84,16 @@ $bulk-actions-offset-slide-in-start: rem(-40px); position: absolute; width: auto; } +} - .Button { - border-radius: 0; - white-space: nowrap; - - &:focus { - z-index: z-index(focused, $bulk-actions-button-stacking-order); - } - - // stylelint-disable-next-line selector-max-specificity - &:last-child:not(:first-child) { - border-radius: 0 border-radius() border-radius() 0; - } - } - - .Button-cancel { - z-index: 0; - flex: 0 0 auto; - margin-left: -1px; - border-radius: 0 border-radius() border-radius() 0; - - &:focus { - z-index: 1; - } - } +.BulkActionButton { + white-space: nowrap; } .CheckableContainer { flex: 1 1 0; } -.Popover { - max-height: resource-list(button-min-height); - margin-left: -1px; - - &:last-child * { - border-radius: 0 border-radius() border-radius() 0; - } -} - -.ActionContent { - display: flex; - align-items: center; -} - -.ActionIcon { - @include recolor-icon(color('ink', 'lighter')); - display: inline-block; - - &:first-child { - margin-right: spacing(extra-tight); - } - - &:last-child { - // This compensates for the disclosure icon, which is substantially - // inset within the viewbox (and makes it look like there is too much - // spacing on the right of the button) - margin-right: -1 * spacing(tight); - } - - &.disabled { - @include recolor-icon(color('ink', 'lightest')); - } -} - .disabled { @include base-button-disabled; cursor: default; diff --git a/src/components/ResourceList/components/BulkActions/BulkActions.tsx b/src/components/ResourceList/components/BulkActions/BulkActions.tsx index 681fb6f58c4..ce80743a5ad 100644 --- a/src/components/ResourceList/components/BulkActions/BulkActions.tsx +++ b/src/components/ResourceList/components/BulkActions/BulkActions.tsx @@ -7,6 +7,7 @@ import {DisableableAction, Action, ActionListSection} from '../../../../types'; import {ActionList} from '../../../ActionList'; import {Popover} from '../../../Popover'; import {Button} from '../../../Button'; +import {ButtonGroup} from '../../../ButtonGroup'; import {EventListener} from '../../../EventListener'; import { withAppProvider, @@ -246,20 +247,14 @@ class BulkActions extends React.PureComponent { ) : null; - const cancelButtonClassName = classNames( - styles.Button, - styles['Button-cancel'], - disabled && styles.disabled, - ); const cancelButton = ( - + ); const numberOfPromotedActionsToRender = this @@ -385,23 +380,25 @@ class BulkActions extends React.PureComponent { className={smallScreenGroupClassName} ref={this.smallScreenGroupNode} > -
- -
+ + - -
- - {allActionsPopover} - {cancelButton} +
+ +
+ + {allActionsPopover} + {cancelButton} +
{paginatedSelectAllMarkup} @@ -410,6 +407,17 @@ class BulkActions extends React.PureComponent { ) : null; + const largeGroupContent = + promotedActionsMarkup || actionsPopover ? ( + + + {promotedActionsMarkup} + {actionsPopover} + + ) : ( + + ); + const largeScreenGroup = smallScreen ? null : ( { >
- - {promotedActionsMarkup} - {actionsPopover} + {largeGroupContent}
{paginatedSelectAllMarkup} diff --git a/src/components/ResourceList/components/BulkActions/components/BulkActionButton/BulkActionButton.tsx b/src/components/ResourceList/components/BulkActions/components/BulkActionButton/BulkActionButton.tsx index df43ecaefe7..2e766bfb20f 100644 --- a/src/components/ResourceList/components/BulkActions/components/BulkActionButton/BulkActionButton.tsx +++ b/src/components/ResourceList/components/BulkActions/components/BulkActionButton/BulkActionButton.tsx @@ -1,13 +1,6 @@ import React, {createRef} from 'react'; -import {CaretDownMinor} from '@shopify/polaris-icons'; - -import {classNames} from '../../../../../../utilities/css'; -import {Icon} from '../../../../../Icon'; -import {UnstyledLink} from '../../../../../UnstyledLink'; import {DisableableAction} from '../../../../../../types'; - -import {handleMouseUpByBlurring} from '../../../../../../utilities/focus'; - +import {Button} from '../../../../../Button'; import styles from '../../BulkActions.scss'; export type BulkActionButtonProps = { @@ -19,7 +12,7 @@ export class BulkActionButton extends React.PureComponent< BulkActionButtonProps, never > { - private bulkActionButton = createRef(); + private bulkActionButton = createRef(); componentDidMount() { const {handleMeasurement} = this.props; @@ -40,49 +33,19 @@ export class BulkActionButton extends React.PureComponent< disabled, } = this.props; - const disclosureIconMarkup = disclosure ? ( - - - - ) : null; - - const contentMarkup = disclosureIconMarkup ? ( - - {content} - {disclosureIconMarkup} - - ) : ( - content - ); - - if (url) { - return ( - + + {content} + + ); } } diff --git a/src/components/ResourceList/components/BulkActions/tests/BulkActions.test.tsx b/src/components/ResourceList/components/BulkActions/tests/BulkActions.test.tsx index 9ece7a8e161..7bb258303e9 100644 --- a/src/components/ResourceList/components/BulkActions/tests/BulkActions.test.tsx +++ b/src/components/ResourceList/components/BulkActions/tests/BulkActions.test.tsx @@ -1,6 +1,7 @@ import React from 'react'; import {Transition, CSSTransition} from '@material-ui/react-transition-group'; import {mountWithAppProvider, findByTestID} from 'test-utilities/legacy'; +import {mountWithApp} from 'test-utilities'; import {Popover} from 'components'; import {CheckableButton} from '../../CheckableButton'; import {BulkActionButton} from '../components'; @@ -379,4 +380,21 @@ describe('', () => { }); }); }); + + describe('buttongroup', () => { + // Since we need to break our component model and reach into ButtonGroup to access the CheckableButton + // and ensure only the first element flex grows, we add this test to ensure the mark-up does not change + it('has the mark-up structure to target the CheckableButton', () => { + const bulkActions = mountWithApp( + , + ); + + const checkableButton = bulkActions! + .find('div', { + className: 'ButtonGroupWrapper', + })! + .domNode!.querySelector('div > div.Item:first-child'); + expect(checkableButton).not.toBeNull(); + }); + }); }); diff --git a/src/components/ResourceList/components/CheckableButton/CheckableButton.scss b/src/components/ResourceList/components/CheckableButton/CheckableButton.scss index 58b0019f15f..38b0669ee0f 100644 --- a/src/components/ResourceList/components/CheckableButton/CheckableButton.scss +++ b/src/components/ResourceList/components/CheckableButton/CheckableButton.scss @@ -9,7 +9,7 @@ $chekbox-label-margin: rem(20px); .CheckableButton { @include text-style-button; - display: inline-flex; + display: flex; align-items: center; min-height: resource-list(button-min-height); min-width: resource-list(button-min-height); @@ -20,10 +20,31 @@ $chekbox-label-margin: rem(20px); user-select: none; text-decoration: none; text-align: left; - background: var(--p-surface-foreground, color('white')); - border: border(dark); - border-radius: border-radius() 0 0 border-radius(); - border-right-color: transparent; + background: transparent; + border: var(--p-override-none, border(dark)); + border-radius: var(--p-border-radius-base, border-radius()) 0 0 + var(--p-border-radius-base, border-radius()); + border-right: none; + width: 100%; + box-shadow: var(--p-override-none, shadow(faint)); + + &.globalTheming { + @include recolor-icon(var(--p-icon-on-surface)); + background: var(--p-neutral-action); + color: var(--p-text-on-surface); + + &:hover { + background: var(--p-neutral-action-hovered); + } + + &:active { + background: var(--p-neutral-action-pressed); + } + // stylelint-disable-next-line selector-max-class + &.CheckableButton-selectMode { + font-weight: 500; + } + } &.CheckableButton-measuring { font-size: font-size(button); @@ -39,16 +60,6 @@ $chekbox-label-margin: rem(20px); @include breakpoint-after(resource-list(breakpoint-small)) { flex: 0 1 auto; - - &:only-child { - border-radius: border-radius(); - border: border(dark); - } - } - - &:hover, - &:active { - border-right-color: transparent; } &:focus { @@ -58,6 +69,12 @@ $chekbox-label-margin: rem(20px); &.CheckableButton-plain { border: border(transparent); border-radius: border-radius(); + box-shadow: none; + background: transparent; + + &:hover { + background: transparent; + } } &.CheckableButton-selectMode { @@ -66,7 +83,7 @@ $chekbox-label-margin: rem(20px); } &.CheckableButton-selected { - color: color('indigo'); + color: var(--p-text-on-surface, color('indigo')); @include breakpoint-after(resource-list(breakpoint-small)) { border-color: color('sky', 'dark'); @@ -86,6 +103,7 @@ $chekbox-label-margin: rem(20px); flex: 1; white-space: nowrap; overflow: hidden; + max-width: 100%; text-overflow: ellipsis; // padding to fix the bottom of letters being cutoff by overflow: hidden padding: rem(1px) 0; diff --git a/src/components/ResourceList/components/CheckableButton/CheckableButton.tsx b/src/components/ResourceList/components/CheckableButton/CheckableButton.tsx index fb6aff5f787..d082efa4b02 100644 --- a/src/components/ResourceList/components/CheckableButton/CheckableButton.tsx +++ b/src/components/ResourceList/components/CheckableButton/CheckableButton.tsx @@ -1,5 +1,6 @@ import React, {useRef, useEffect} from 'react'; import {CheckboxHandles} from '../../../../types'; +import {useFeatures} from '../../../../utilities/features'; import {classNames} from '../../../../utilities/css'; import {Checkbox} from '../../../Checkbox'; import { @@ -33,6 +34,7 @@ export function CheckableButton({ smallScreen, }: CheckableButtonProps) { const checkBoxRef = useRef(null); + const {unstableGlobalTheming = false} = useFeatures(); const {registerCheckableButtons} = React.useContext(ResourceListContext); @@ -51,9 +53,14 @@ export function CheckableButton({ }, [currentKey, registerCheckableButtons]); const className = plain - ? classNames(styles.CheckableButton, styles['CheckableButton-plain']) + ? classNames( + styles.CheckableButton, + styles['CheckableButton-plain'], + unstableGlobalTheming && styles.globalTheming, + ) : classNames( styles.CheckableButton, + unstableGlobalTheming && styles.globalTheming, selectMode && styles['CheckableButton-selectMode'], selected && styles['CheckableButton-selected'], measuring && styles['CheckableButton-measuring'], diff --git a/src/components/ResourceList/components/CheckableButton/tests/CheckableButton.test.tsx b/src/components/ResourceList/components/CheckableButton/tests/CheckableButton.test.tsx index d4408bfffc9..052d02bda50 100644 --- a/src/components/ResourceList/components/CheckableButton/tests/CheckableButton.test.tsx +++ b/src/components/ResourceList/components/CheckableButton/tests/CheckableButton.test.tsx @@ -1,5 +1,6 @@ import React from 'react'; import {mountWithAppProvider} from 'test-utilities/legacy'; +import {mountWithApp} from 'test-utilities'; import {Checkbox} from 'components'; import {CheckableButton} from '../CheckableButton'; import {Key} from '../../../../../types'; @@ -95,4 +96,30 @@ describe('', () => { expect(spy).toHaveBeenCalled(); }); }); + + describe('globalTheming', () => { + it('adds a global theming class when global theming is enabled and the button is selected', () => { + const checkableButton = mountWithApp( + , + { + features: {unstableGlobalTheming: true}, + }, + ); + expect(checkableButton).toContainReactComponent('div', { + className: 'CheckableButton globalTheming CheckableButton-selected', + }); + }); + + it('adds a global theming class when global theming is enabled and the button is not selected', () => { + const checkableButton = mountWithApp( + , + { + features: {unstableGlobalTheming: true}, + }, + ); + expect(checkableButton).toContainReactComponent('div', { + className: 'CheckableButton CheckableButton-plain globalTheming', + }); + }); + }); }); diff --git a/src/components/ResourceList/tests/ResourceList.test.tsx b/src/components/ResourceList/tests/ResourceList.test.tsx index 46d29090b29..9c7e41a9458 100644 --- a/src/components/ResourceList/tests/ResourceList.test.tsx +++ b/src/components/ResourceList/tests/ResourceList.test.tsx @@ -878,7 +878,7 @@ describe('', () => { />, ); - trigger(resourceList.find(Button), 'onClick'); + trigger(resourceList.find(Button).first(), 'onClick'); const selectAllCheckableButton = bulkActionsCheckableButton( resourceList, @@ -1110,7 +1110,7 @@ function setSmallScreen() { Object.defineProperty(window, 'innerWidth', { configurable: true, writable: true, - value: 458, + value: 457, }); } diff --git a/tests/build.test.js b/tests/build.test.js index 701e753d96c..22a88f69be3 100644 --- a/tests/build.test.js +++ b/tests/build.test.js @@ -50,7 +50,7 @@ describe('build', () => { it('generates fully namespaced CSS for nested components', () => { expect(fs.readFileSync('./styles/components.scss', 'utf8')).toMatch( - '.Polaris-ResourceList-BulkActions__Button{', + '.Polaris-ResourceList-BulkActions__BulkActionButton{', ); });