Update to button group css

Shopify · Nov 21, 2019 · fed5e46 · fed5e46
1 parent 8c771cc
commit fed5e46
Show file tree

Hide file tree

Showing 7 changed files with 127 additions and 79 deletions.
diff --git a/UNRELEASED.md b/UNRELEASED.md
@@ -7,6 +7,7 @@
 - Added `external` prop to `ResourceList` ([#2408](https://github.com/Shopify/polaris-react/pull/2408))
 - 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))
+- Moved `Button` styles from the `Buttongroup` CSS file to the `Button` CSS file ([#2441](https://github.com/Shopify/polaris-react/pull/2441))
 
 ### Bug fixes
 
@@ -33,5 +34,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
diff --git a/src/components/Button/Button.scss b/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;

diff --git a/src/components/ButtonGroup/ButtonGroup.scss b/src/components/ButtonGroup/ButtonGroup.scss
@@ -37,8 +37,6 @@ $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);
@@ -48,51 +46,24 @@ $item-spacing: spacing(tight);
     &: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();
-    }
   }
 
   .Item-focused {
     z-index: z-index(focused, $stacking-order);
   }
-}
-
-.fullWidth {
-  .Item {
-    flex: 1 1 auto;
 
-    // stylelint-disable-next-line selector-max-combinators
-    > * {
-      @include button-full-width;
+  &.globalTheming {
+    .Item {
+      // stylelint-disable-next-line selector-max-class, selector-max-specificity
+      &:not(:first-child) {
+        margin-left: spacing(extra-tight);
+      }
     }
   }
 }
 
-.connectedTop {
+.fullWidth {
   .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;
-    }
+    flex: 1 1 auto;
   }
 }
diff --git a/src/components/ButtonGroup/ButtonGroup.tsx b/src/components/ButtonGroup/ButtonGroup.tsx
@@ -1,5 +1,6 @@
 import React from 'react';
 import {classNames} from '../../utilities/css';
+import {useFeatures} from '../../utilities/features';
 import {elementChildren} from '../../utilities/components';
 import {Item} from './components';
 import styles from './ButtonGroup.scss';
@@ -21,16 +22,27 @@ export function ButtonGroup({
   fullWidth,
   connectedTop,
 }: ButtonGroupProps) {
+  const {unstableGlobalTheming = false} = useFeatures();
+
   const className = classNames(
     styles.ButtonGroup,
+    unstableGlobalTheming && styles.globalTheming,
     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>
+  );
 }
diff --git a/src/components/ButtonGroup/components/Item/Item.tsx b/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>
+  );
 }
diff --git a/src/components/ButtonGroup/components/Item/tests/Item.test.tsx b/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',
+      });
+    });
   });
 });
diff --git a/src/components/ButtonGroup/tests/ButtonGroup.test.tsx b/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);
+    });
   });
 });