[ButtonGroup] Add theme to ButtonGroup and move CSS to Button

UNRELEASED.md

@@ -8,6 +8,7 @@
 - Added `onMouseEnter` and `onTouchStart` props to `Button` ([#2409](https://github.com/Shopify/polaris-react/pull/2409))
 - Added `ariaHaspopup` prop to `Popover` ([#2248](https://github.com/Shopify/polaris-react/pull/2248))
 - Fixed an accessibility issue where the `Form` implicit submit was still accessible via keyboard ([#2447](https://github.com/Shopify/polaris-react/pull/2447))
+- Moved `Button` styles from the `Buttongroup` CSS file to the `Button` CSS file ([#2441](https://github.com/Shopify/polaris-react/pull/2441))
 
 ### Bug fixes
 
@@ -37,5 +38,6 @@
 ### Code quality
 
 - 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))
 
 ### Deprecations

src/components/Button/Button.scss

@@ -53,13 +53,13 @@ $partial-button-filled-pressed-box-shadow: inset 0 0 0 0 transparent,
     transition-duration: duration(fast);
     background: linear-gradient(to bottom, $color-light, $color-light);
     border-color: $color-dark;
-    box-shadow: $partial-button-filled-pressed-box-shadow$color-dark;
+    box-shadow: $partial-button-filled-pressed-box-shadow $color-dark;
   }
 
   &:active {
     background: linear-gradient(to bottom, $color-lightest, $color-lightest);
     border-color: $color-dark;
-    box-shadow: $partial-button-filled-pressed-box-shadow$color-darkest;
+    box-shadow: $partial-button-filled-pressed-box-shadow $color-darkest;
   }
 }
 
@@ -70,7 +70,37 @@ $partial-button-filled-pressed-box-shadow: inset 0 0 0 0 transparent,
     @include base-button-disabled;
   }
 }
+// stylelint-disable selector-max-combinators
+[data-buttongroup-segmented] {
+  .Button {
+    border-radius: 0;
+  }
+  > :first-child .Button {
+    border-top-left-radius: var(--p-border-radius-base, border-radius());
+    border-bottom-left-radius: var(--p-border-radius-base, border-radius());
+  }
+  > :last-child .Button {
+    border-top-right-radius: var(--p-border-radius-base, border-radius());
+    border-bottom-right-radius: var(--p-border-radius-base, border-radius());
+  }
+}
+
+[data-buttongroup-connected-top] {
+  > :first-child .Button {
+    border-top-left-radius: 0;
+  }
+
+  > :last-child .Button {
+    border-top-right-radius: 0;
+  }
+}
 
+[data-buttongroup-full-width] {
+  .Button {
+    @include button-full-width;
+  }
+}
+// stylelint-enable selector-max-combinators
 .Content {
   @include text-style-button;
   position: relative;
@@ -133,6 +163,10 @@ $partial-button-filled-pressed-box-shadow: inset 0 0 0 0 transparent,
   @include button-filled(color('indigo'), color('indigo', 'dark'));
   @include recolor-icon(color('white'));
 
+  &.globalTheming {
+    @include recolor-icon(var(--p-icon-on-branded));
+  }
+
   &.disabled {
     @include button-filled-disabled(color('indigo'));
   }
@@ -494,6 +528,11 @@ $partial-button-filled-pressed-box-shadow: inset 0 0 0 0 transparent,
   .Icon:last-child {
     margin-right: rem(-4px);
   }
+  // stylelint-disable selector-max-class, selector-max-combinators, selector-max-specificity
+  .Icon + .Icon:last-child {
+    margin-left: 0;
+  }
+  // stylelint-enable selector-max-class, selector-max-combinators, selector-max-specificity
 
   .Icon:only-child {
     margin-right: 0;

src/components/ButtonGroup/ButtonGroup.scss

@@ -37,33 +37,14 @@ $item-spacing: spacing(tight);
   margin-top: 0;
   margin-left: 0;
 
-  // This is a violation of our component model, but it’s the cleanest
-  // way to remove the border radii on connected elements.
   .Item {
     position: relative;
     z-index: z-index(item, $stacking-order);
     margin-top: 0;
     margin-left: 0;
 
     &:not(:first-child) {
-      margin-left: -(border-width());
-    }
-
-    // stylelint-disable-next-line selector-max-combinators
-    > * {
-      border-radius: 0;
-    }
-
-    // stylelint-disable-next-line selector-max-combinators
-    &:first-child > * {
-      border-top-left-radius: border-radius();
-      border-bottom-left-radius: border-radius();
-    }
-
-    // stylelint-disable-next-line selector-max-combinators
-    &:last-child > * {
-      border-top-right-radius: border-radius();
-      border-bottom-right-radius: border-radius();
+      margin-left: var(--p-button-group-item-spacing, -1 * border-width());
     }
   }
 
@@ -75,24 +56,5 @@ $item-spacing: spacing(tight);
 .fullWidth {
   .Item {
     flex: 1 1 auto;
-
-    // stylelint-disable-next-line selector-max-combinators
-    > * {
-      @include button-full-width;
-    }
-  }
-}
-
-.connectedTop {
-  .Item {
-    // stylelint-disable-next-line selector-max-combinators
-    &:first-child > * {
-      border-top-left-radius: 0;
-    }
-
-    // stylelint-disable-next-line selector-max-combinators
-    &:last-child > * {
-      border-top-right-radius: 0;
-    }
   }
 }

src/components/ButtonGroup/ButtonGroup.tsx

@@ -25,12 +25,20 @@ export function ButtonGroup({
     styles.ButtonGroup,
     segmented && styles.segmented,
     fullWidth && styles.fullWidth,
-    connectedTop && styles.connectedTop,
   );
 
   const contents = elementChildren(children).map((child, index) => (
     <Item button={child} key={index} />
   ));
 
-  return <div className={className}>{contents}</div>;
+  return (
+    <div
+      className={className}
+      data-buttongroup-segmented={segmented}
+      data-buttongroup-connected-top={connectedTop}
+      data-buttongroup-full-width={fullWidth}
+    >
+      {contents}
+    </div>
+  );
 }

src/components/ButtonGroup/components/Item/Item.tsx

@@ -1,47 +1,32 @@
 import React from 'react';
-
+import {useForcibleToggle} from '../../../../utilities/use-toggle';
 import {classNames} from '../../../../utilities/css';
-import {ButtonProps} from '../../../Button';
 
 import styles from '../../ButtonGroup.scss';
 
 export interface ItemProps {
-  button: React.ReactElement<ButtonProps>;
-}
-
-interface State {
-  focused: boolean;
+  button: React.ReactElement;
 }
 
-export class Item extends React.PureComponent<ItemProps, State> {
-  state: State = {focused: false};
-
-  render() {
-    const {button} = this.props;
-    const {focused} = this.state;
-
-    const className = classNames(
-      styles.Item,
-      focused && styles['Item-focused'],
-      button.props.plain && styles['Item-plain'],
-    );
-
-    return (
-      <div
-        className={className}
-        onFocus={this.handleFocus}
-        onBlur={this.handleBlur}
-      >
-        {button}
-      </div>
-    );
-  }
-
-  private handleFocus = () => {
-    this.setState({focused: true});
-  };
-
-  private handleBlur = () => {
-    this.setState({focused: false});
-  };
+export function Item({button}: ItemProps) {
+  const [
+    focused,
+    {forceTrue: forceTrueFocused, forceFalse: forceFalseFocused},
+  ] = useForcibleToggle(false);
+
+  const className = classNames(
+    styles.Item,
+    focused && styles['Item-focused'],
+    button.props.plain && styles['Item-plain'],
+  );
+
+  return (
+    <div
+      className={className}
+      onFocus={forceTrueFocused}
+      onBlur={forceFalseFocused}
+    >
+      {button}
+    </div>
+  );
 }

src/components/ButtonGroup/components/Item/tests/Item.test.tsx

@@ -1,5 +1,6 @@
 import React from 'react';
 import {mountWithAppProvider} from 'test-utilities/legacy';
+import {mountWithApp} from 'test-utilities';
 import {Button} from 'components';
 import {Item} from '../Item';
 
@@ -10,5 +11,14 @@ describe('<Item />', () => {
       const item = mountWithAppProvider(<Item button={button} />);
       expect(item.contains(button)).toBeTruthy();
     });
+
+    it('sets focus styles when focused', () => {
+      const button = <Button>Button text</Button>;
+      const item = mountWithApp(<Item button={button} />);
+      item.find('div')!.trigger('onFocus');
+      expect(item.find('div')).toHaveReactProps({
+        className: 'Item Item-focused',
+      });
+    });
   });
 });

src/components/ButtonGroup/tests/ButtonGroup.test.tsx

@@ -1,5 +1,7 @@
 import React from 'react';
 import {mountWithAppProvider} from 'test-utilities/legacy';
+import {mountWithApp} from 'test-utilities';
+
 import {Button} from 'components';
 import {Item} from '../components';
 import {ButtonGroup} from '../ButtonGroup';
@@ -25,5 +27,41 @@ describe('<ButtonGroup />', () => {
       );
       expect(buttonGroup.find(Item).prop('button').key).toContain(key);
     });
+
+    it('adds a data-buttongroup-segmented to the outter div when segmented', () => {
+      const buttonGroup = mountWithApp(
+        <ButtonGroup segmented>
+          <Button />
+        </ButtonGroup>,
+      );
+      const selector: any = {
+        'data-buttongroup-segmented': true,
+      };
+      expect(buttonGroup).toContainReactComponent('div', selector);
+    });
+
+    it('adds a data-buttongroup-full-width to the outter div when fullWidth', () => {
+      const buttonGroup = mountWithApp(
+        <ButtonGroup fullWidth>
+          <Button />
+        </ButtonGroup>,
+      );
+      const selector: any = {
+        'data-buttongroup-full-width': true,
+      };
+      expect(buttonGroup).toContainReactComponent('div', selector);
+    });
+
+    it('adds a data-buttongroup-connected-top to the outter div when connectedTop', () => {
+      const buttonGroup = mountWithApp(
+        <ButtonGroup connectedTop>
+          <Button />
+        </ButtonGroup>,
+      );
+      const selector: any = {
+        'data-buttongroup-connected-top': true,
+      };
+      expect(buttonGroup).toContainReactComponent('div', selector);
+    });
   });
 });

src/utilities/theme/utils.ts

@@ -128,6 +128,7 @@ function overrides() {
       '-1px 0px 20px var(--p-shadow-from-ambient-light), 0px 1px 5px var(--p-shadow-from-direct-light)',
     modalShadow:
       '0px 6px 32px var(--p-shadow-from-ambient-light), 0px 1px 6px var(--p-shadow-from-direct-light)',
+    buttonGroupItemSpacing: rem('2px'),
   };
 }
 

yarn.lock

@@ -1479,12 +1479,12 @@
     "@shopify/ast-utilities" "^0.0.6"
     webpack-virtual-modules "^0.1.12"
 
-"@shopify/react-testing@^1.7.8":
-  version "1.7.8"
-  resolved "https://registry.yarnpkg.com/@shopify/react-testing/-/react-testing-1.7.8.tgz#64b79d8b1cc4791d8e77e3ad5c06dfb7380c88b7"
-  integrity sha512-P4CNcYPjX6OKMe4f6LfyTM0UWlQqinBSoFIWPrWRwAI0qnwKRYnP81WdUANpmeY01SxAIp/WwjKTgpg3sdTnoQ==
+"@shopify/react-testing@^1.8.0":
+  version "1.8.6"
+  resolved "https://registry.yarnpkg.com/@shopify/react-testing/-/react-testing-1.8.6.tgz#910a90910d443812dba6f4067a7e55ff44fe4a14"
+  integrity sha512-9QagkYVOUiIjsHQG6yuGITUZErMDaKjpARPqSCa3tiIASZuRpNdXIy0B881zSWlKsVahV/u+8Z+uRl7bVL698w==
   dependencies:
-    "@shopify/useful-types" "^2.0.1"
+    "@shopify/useful-types" "^2.1.2"
     jest-matcher-utils "^24.5.0"
     react-reconciler "^0.20.2"
 
@@ -1613,10 +1613,10 @@
     "@types/react" ">=16.4.0"
     tslib "^1.9.3"
 
-"@shopify/useful-types@^2.0.1":
-  version "2.0.1"
-  resolved "https://registry.yarnpkg.com/@shopify/useful-types/-/useful-types-2.0.1.tgz#df27503acb26249b83be84e2298186bf7a0194fc"
-  integrity sha512-dmBg7FIbONo+KN4+q9nhiLNb9qIFMkWcar5yUJIipTtNLtPKyaDvHCA6oL7ofzvtq+8uFD6jw5kbswXCutJY9A==
+"@shopify/useful-types@^2.1.2":
+  version "2.1.2"
+  resolved "https://registry.yarnpkg.com/@shopify/useful-types/-/useful-types-2.1.2.tgz#b489005c8ee72c0d22c89bb1114aca2aaf0059b4"
+  integrity sha512-2gBvMeY5J6nyJBm3mLZS+P4LOT3aVi0kE1OIYNfCbonr8jiWquBI4YqJ4n2nhG1rmSW1bizSe0xz0ZZXgfGkBg==
   dependencies:
     tslib "^1.9.3"