Merge pull request #2467 from Shopify/cs-bulkactions

[BulkActions] Add theming to BulkAction and remove custom ButtonGroup from ResourceList

UNRELEASED.md

@@ -46,5 +46,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

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 & {

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

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;

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<CombinedProps, State> {
         </div>
       ) : null;
 
-    const cancelButtonClassName = classNames(
-      styles.Button,
-      styles['Button-cancel'],
-      disabled && styles.disabled,
-    );
     const cancelButton = (
-      <button
-        className={cancelButtonClassName}
+      <Button
         onClick={this.setSelectMode.bind(this, false)}
         testID="btn-cancel"
         disabled={disabled}
       >
         {intl.translate('Polaris.Common.cancel')}
-      </button>
+      </Button>
     );
 
     const numberOfPromotedActionsToRender = this
@@ -385,23 +380,25 @@ class BulkActions extends React.PureComponent<CombinedProps, State> {
               className={smallScreenGroupClassName}
               ref={this.smallScreenGroupNode}
             >
-              <div className={styles.ButtonGroup}>
-                <CSSTransition
-                  findDOMNode={this.findCheckableWrapperNode}
-                  in={selectMode}
-                  timeout={durationBase}
-                  classNames={slideClasses}
-                  appear
-                >
-                  <div
-                    className={styles.CheckableContainer}
-                    ref={this.checkableWrapperNode}
+              <div className={styles.ButtonGroupWrapper}>
+                <ButtonGroup segmented>
+                  <CSSTransition
+                    findDOMNode={this.findCheckableWrapperNode}
+                    in={selectMode}
+                    timeout={durationBase}
+                    classNames={slideClasses}
+                    appear={!selectMode}
                   >
-                    <CheckableButton {...checkableButtonProps} smallScreen />
-                  </div>
-                </CSSTransition>
-                {allActionsPopover}
-                {cancelButton}
+                    <div
+                      className={styles.CheckableContainer}
+                      ref={this.checkableWrapperNode}
+                    >
+                      <CheckableButton {...checkableButtonProps} smallScreen />
+                    </div>
+                  </CSSTransition>
+                  {allActionsPopover}
+                  {cancelButton}
+                </ButtonGroup>
               </div>
               {paginatedSelectAllMarkup}
             </div>
@@ -410,6 +407,17 @@ class BulkActions extends React.PureComponent<CombinedProps, State> {
       </Transition>
     ) : null;
 
+    const largeGroupContent =
+      promotedActionsMarkup || actionsPopover ? (
+        <ButtonGroup segmented>
+          <CheckableButton {...checkableButtonProps} />
+          {promotedActionsMarkup}
+          {actionsPopover}
+        </ButtonGroup>
+      ) : (
+        <CheckableButton {...checkableButtonProps} />
+      );
+
     const largeScreenGroup = smallScreen ? null : (
       <Transition
         timeout={0}
@@ -432,12 +440,10 @@ class BulkActions extends React.PureComponent<CombinedProps, State> {
             >
               <EventListener event="resize" handler={this.handleResize} />
               <div
-                className={styles.ButtonGroup}
+                className={styles.ButtonGroupWrapper}
                 ref={this.setLargeScreenButtonsNode}
               >
-                <CheckableButton {...checkableButtonProps} />
-                {promotedActionsMarkup}
-                {actionsPopover}
+                {largeGroupContent}
               </div>
               {paginatedSelectAllMarkup}
             </div>

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<HTMLButtonElement>();
+  private bulkActionButton = createRef<HTMLDivElement>();
 
   componentDidMount() {
     const {handleMeasurement} = this.props;
@@ -40,49 +33,19 @@ export class BulkActionButton extends React.PureComponent<
       disabled,
     } = this.props;
 
-    const disclosureIconMarkup = disclosure ? (
-      <span className={styles.ActionIcon}>
-        <Icon source={CaretDownMinor} />
-      </span>
-    ) : null;
-
-    const contentMarkup = disclosureIconMarkup ? (
-      <span className={styles.ActionContent}>
-        <span>{content}</span>
-        {disclosureIconMarkup}
-      </span>
-    ) : (
-      content
-    );
-
-    if (url) {
-      return (
-        <UnstyledLink
+    return (
+      <div className={styles.BulkActionButton} ref={this.bulkActionButton}>
+        <Button
           external={external}
           url={url}
-          onMouseUp={handleMouseUpByBlurring}
-          className={styles.Button}
           aria-label={accessibilityLabel}
+          onClick={onAction}
+          disabled={disabled}
+          disclosure={disclosure}
         >
-          {contentMarkup}
-        </UnstyledLink>
-      );
-    }
-
-    const className = classNames(styles.Button, disabled && styles.disabled);
-
-    return (
-      <button
-        className={className}
-        onClick={onAction}
-        onMouseUp={handleMouseUpByBlurring}
-        aria-label={accessibilityLabel}
-        type="button"
-        disabled={disabled}
-        ref={this.bulkActionButton}
-      >
-        {contentMarkup}
-      </button>
+          {content}
+        </Button>
+      </div>
     );
   }
 }

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('<BulkActions />', () => {
       });
     });
   });
+
+  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(
+        <BulkActions {...bulkActionProps} selectMode smallScreen />,
+      );
+
+      const checkableButton = bulkActions!
+        .find('div', {
+          className: 'ButtonGroupWrapper',
+        })!
+        .domNode!.querySelector('div > div.Item:first-child');
+      expect(checkableButton).not.toBeNull();
+    });
+  });
 });