refactor(media): remove menu wrapper div, pass props to button

box · Jul 16, 2019 · 53c7b00 · 53c7b00
1 parent aa58dc2
commit 53c7b00
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 28 deletions.
diff --git a/src/components/dropdown-menu/DropdownMenu.js b/src/components/dropdown-menu/DropdownMenu.js
@@ -171,6 +171,7 @@ class DropdownMenu extends React.Component<Props, State> {
             constrainToScrollParent,
             constrainToWindow,
             className,
+            ...rest
         } = this.props;
         const { isOpen, initialFocusIndex } = this.state;
 
@@ -184,6 +185,7 @@ class DropdownMenu extends React.Component<Props, State> {
         const menu = elements[1];
 
         const menuButtonProps: Object = {
+            ...rest,
             id: this.menuButtonID,
             key: this.menuButtonID,
             onClick: this.handleButtonClick, // NOTE: Overrides button's handler

diff --git a/src/components/dropdown-menu/__tests__/DropdownMenu-test.js b/src/components/dropdown-menu/__tests__/DropdownMenu-test.js
@@ -128,6 +128,19 @@ describe('components/dropdown-menu/DropdownMenu', () => {
             expect(menu.prop('aria-labelledby')).toEqual(instance.menuButtonID);
         });
 
+        test('should pass extra props to button', () => {
+            const extraProps = { foo: 'bar', 'aria-label': 'open the menu' };
+            const wrapper = shallow(
+                <DropdownMenu {...extraProps}>
+                    <FakeButton />
+                    <FakeMenu />
+                </DropdownMenu>,
+            );
+
+            const button = wrapper.find(FakeButton);
+            expect(button.props()).toEqual(expect.objectContaining(extraProps));
+        });
+
         test('should render TetherComponent with correct props with correct default values', () => {
             const wrapper = shallow(
                 <DropdownMenu>

diff --git a/src/components/media/Media.scss b/src/components/media/Media.scss
@@ -28,6 +28,14 @@
 // float is used to reflow content instead of pushing it to the left
 .bdl-Media-menu {
     float: right;
+}
+
+// overrides for .btn-plain's margin resets on focus states
+.bdl-Media-menu,
+.bdl-Media-menu:active,
+.bdl-Media-menu:hover,
+.bdl-Media-menu:focus {
+    float: right;
     margin-bottom: 5px;
     margin-left: 5px;
 }
diff --git a/src/components/media/MediaMenu.js b/src/components/media/MediaMenu.js
@@ -11,23 +11,19 @@ import './Media.scss';
 type Props = {
     /** Child elements */
     children: React.Node,
-    /** Additional class names */
+    /** Additional class names for the menu button */
     className?: string,
     /** is the dropdown menu button disabled */
     isDisabled: boolean,
-    /** aria-label prop for button icon */
-    label?: string,
 };
 
-const MediaMenu = ({ className, children, isDisabled, label, ...rest }: Props) => (
-    <div className={classnames('bdl-Media-menu', className)} {...rest}>
-        <DropdownMenu constrainToScrollParent isRightAligned>
-            <PlainButton isDisabled={isDisabled} type="button" aria-label={label}>
-                <IconEllipsis color={bdlGray62} height={16} width={16} />
-            </PlainButton>
-            <Menu>{children}</Menu>
-        </DropdownMenu>
-    </div>
+const MediaMenu = ({ className, children, isDisabled, ...rest }: Props) => (
+    <DropdownMenu constrainToScrollParent isRightAligned {...rest}>
+        <PlainButton isDisabled={isDisabled} className={classnames('bdl-Media-menu', className)} type="button">
+            <IconEllipsis color={bdlGray62} height={16} width={16} />
+        </PlainButton>
+        <Menu>{children}</Menu>
+    </DropdownMenu>
 );
 
 MediaMenu.defaultProps = {

diff --git a/src/components/media/__tests__/MediaMenu-test.js b/src/components/media/__tests__/MediaMenu-test.js
@@ -3,9 +3,16 @@ import React from 'react';
 import MediaMenu from '../MediaMenu';
 
 describe('components/Media/MediaMenu', () => {
-    test('label prop adds aria-label attribute to menu button', () => {
-        const label = 'Open options';
-        const wrapper = shallow(<MediaMenu label={label} />);
-        expect(wrapper.find('PlainButton').prop('aria-label')).toBe(label);
+    test('props are spread onto button', () => {
+        const extraProps = {
+            'aria-label': 'label for menu',
+            'data-testid': 'a-menu',
+            'resin-target': 'my-menu',
+        };
+        const className = 'foo';
+        const wrapper = mount(<MediaMenu className={className} {...extraProps} />);
+
+        expect(wrapper.find('PlainButton').props()).toEqual(expect.objectContaining(extraProps));
+        expect(wrapper.find('PlainButton').prop('className')).toBe(`bdl-Media-menu ${className}`);
     });
 });
diff --git a/src/components/media/__tests__/__snapshots__/Media-test.js.snap b/src/components/media/__tests__/__snapshots__/Media-test.js.snap
@@ -15,19 +15,15 @@ exports[`components/Media compound component 1`] = `
   <div
     class="bdl-Media-body"
   >
-    <div
-      class="bdl-Media-menu"
+    <button
+      aria-expanded="false"
+      aria-haspopup="true"
+      class="btn-plain bdl-Media-menu"
+      id="menubutton2"
+      type="button"
     >
-      <button
-        aria-expanded="false"
-        aria-haspopup="true"
-        class="btn-plain "
-        id="menubutton2"
-        type="button"
-      >
-        <icon-ellipsis />
-      </button>
-    </div>
+      <icon-ellipsis />
+    </button>
     <div>
       <b>
         Yo Yo Ma