Repository: mapbox/mr-ui
Pull request #31 (2018-08-08 02:09:13)

Remove transformClasses from Button, add elementAttributes
Also adds a `shallowEqualObjects` utility that I didn't end up using but we might want to keep around: could prove handy. If it doesn't, we'll delete it.


Patch:
diff --git a/src/components/button/button.js b/src/components/button/button.js
index b5a56f5..4106830 100644
--- a/src/components/button/button.js
+++ b/src/components/button/button.js
@@ -2,25 +2,6 @@ import React from 'react';
 import classnames from 'classnames';
 import PropTypes from 'prop-types';
 import xtend from 'xtend';
-import omit from '../utils/omit';
-
-// This list must be kept in sync with the propTypes.
-// It's used to identify additional props that should be passed directly
-// to the element.
-const propNames = [
-  'block',
-  'children',
-  'color',
-  'component',
-  'corners',
-  'href',
-  'onClick',
-  'outline',
-  'size',
-  'transformClasses',
-  'variant',
-  'width'
-];
 
 /**
  * A good-looking button!
@@ -31,11 +12,6 @@ const propNames = [
  *
  * If you'd like to put an icon before or after the text of your button,
  * use [IconText](#mbxicontext) for the content.
- *
- * Any props you provide other than those documented here are passed through
- * directly to the element itself. This can be useful if you want to disable
- * the button, assign an ID for testing, add an ARIA attribute, toss in some
- * custom style properties, etc.
  */
 class Button extends React.Component {
   render() {
@@ -70,22 +46,25 @@ class Button extends React.Component {
 
     const universalProps = xtend(
       {
-        className: props.transformClasses(classes),
+        className: classes,
         onClick: props.onClick,
         children: props.children
       },
-      omit(props, propNames)
+      props.passthroughProps
     );
 
+    // "disabled" is not a valid attributes for anchors.
+    const buttonProps = xtend(universalProps, { disabled: props.disabled });
+
     if (props.component) {
-      return React.createElement(props.component, universalProps);
+      return React.createElement(props.component, buttonProps);
     }
 
     if (props.href) {
       return <a href={props.href} {...universalProps} />;
     }
 
-    return <button type="button" {...universalProps} />;
+    return <button type="button" {...buttonProps} />;
   }
 }
 
@@ -232,23 +211,30 @@ Button.propTypes = {
    */
   block: PropTypes.bool,
   /**
-   * A callback for transforming the class list. Receives the standard class
-   * list (based on your other props) as an argument; it must return
-   * the transformed class list.
+   * Is it disbaled?
    */
-  transformClasses: PropTypes.func,
+  disabled: PropTypes.bool,
   /**
    * An alternate component to render in the style of a button. If the value is
    * a string, it must refer to a DOM element, like `"div"`. Otherwise, it
    * can be a React component.
    */
-  component: PropTypes.oneOfType([PropTypes.func, PropTypes.string])
+  component: PropTypes.oneOfType([PropTypes.func, PropTypes.string]),
+  /**
+   * An object of props that you want to pass through to the element that
+   * Button renders.  This can be useful if you want to disable
+   * the button, assign an ID for testing, add an ARIA attribute, toss in some
+   * custom style properties, etc.
+   */
+  passthroughProps: PropTypes.objectOf(
+    PropTypes.oneOfType([PropTypes.string, PropTypes.bool, PropTypes.number])
+  )
 };
 
 Button.defaultProps = {
   variant: 'primary',
   block: false,
-  transformClasses: x => x
+  disabled: false
 };
 
 export default Button;
diff --git a/src/components/utils/README.md b/src/components/utils/README.md
index 606a291..7813508 100644
--- a/src/components/utils/README.md
+++ b/src/components/utils/README.md
@@ -10,6 +10,26 @@ import getWindow from '@mapbox/mr-ui/utils/get-window';
 
 Wraps `window` to make it easy to mock aspects of the browser environment while testing.
 
+## shallowEqualObjects
+
+```js
+import shallowEqualObjects from '@mapbox/mr-ui/utils/shallow-equal-objects';
+
+shallowEqualObjects(
+  a: Object,
+  b: Object,
+  allowedObjectKeys: Array<string>
+): boolean;
+```
+
+Returns `true` if every property of `a` is equal to (with `===`) every property of `b`.
+
+Unlike some other shallow-compare functions, this one will *not* return true if `a === b`. Instead, it *only* evaluates based on the values of the object's properties.
+
+This check is *not* recursive. But `allowObjectKeys` can be used to add *one level* of depth to the check: properties whose keys are in the `allowedObjectKeys` array will themselves be checked with `shallowEqualObjects`, instead of just `===`.
+
+If you try to `shallowEqualObjects` to compare objects with values that are not primitives or functions and do not match an exception in `allowObjectKeys`, an error will be thrown.
+
 ## maybeAddPeriod
 
 ```js
diff --git a/src/components/utils/shallow-equal-objects.js b/src/components/utils/shallow-equal-objects.js
new file mode 100644
index 0000000..ae505fe
--- /dev/null
+++ b/src/components/utils/shallow-equal-objects.js
@@ -0,0 +1,33 @@
+export default function shallowEqualObjects(
+  objA,
+  objB,
+  allowedObjectKeys = []
+) {
+  const aKeys = Object.keys(objA);
+  const bKeys = Object.keys(objB);
+  const len = aKeys.length;
+
+  if (bKeys.length !== len) {
+    return false;
+  }
+
+  for (let i = 0; i < len; i++) {
+    const key = aKeys[i];
+    const aVal = objA[key];
+    const bVal = objB[key];
+    if (allowedObjectKeys.indexOf(key) !== -1) {
+      return shallowEqualObjects(aVal, bVal);
+    }
+
+    if (typeof aVal === 'object') {
+      throw new Error(
+        'shallowEqualObjects can only compare objects whose values are primitives or functions, unless you allow for an object value with the allowedObjectKeys argument'
+      );
+    }
+    if (aVal !== bVal) {
+      return false;
+    }
+  }
+
+  return true;
+}


Test patch:
diff --git a/src/components/button/__tests__/__snapshots__/button.test.js.snap b/src/components/button/__tests__/__snapshots__/button.test.js.snap
index c8c2149..9c6485c 100644
--- a/src/components/button/__tests__/__snapshots__/button.test.js.snap
+++ b/src/components/button/__tests__/__snapshots__/button.test.js.snap
@@ -3,6 +3,7 @@
 exports[`AppPrimary renders as expected 1`] = `
 <button
   className="btn btn--gray round h24 px6 py3 txt-xs"
+  disabled={false}
   onClick={[MockFunction]}
   type="button"
 >
@@ -13,6 +14,7 @@ exports[`AppPrimary renders as expected 1`] = `
 exports[`AppSecondary renders as expected 1`] = `
 <button
   className="btn btn--gray btn--stroke round h24 px6 py3 txt-xs"
+  disabled={false}
   onClick={[MockFunction]}
   type="button"
 >
@@ -23,6 +25,7 @@ exports[`AppSecondary renders as expected 1`] = `
 exports[`Destructive renders as expected 1`] = `
 <button
   className="btn btn--red round-full py12 px24 txt-s"
+  disabled={false}
   onClick={[MockFunction]}
   type="button"
 >
@@ -43,6 +46,7 @@ exports[`Disabled renders as expected 1`] = `
 exports[`Discouraging renders as expected 1`] = `
 <button
   className="btn btn--gray btn--stroke btn--stroke--2 round-full py12 px24 txt-s"
+  disabled={false}
   onClick={[MockFunction]}
   type="button"
 >
@@ -50,10 +54,11 @@ exports[`Discouraging renders as expected 1`] = `
 </button>
 `;
 
-exports[`Div styled like a medium destructive button with some extended props and transformed classes renders as expected 1`] = `
+exports[`Div styled like a medium destructive button with some elementAttributes renders as expected 1`] = `
 <div
-  className="btn btn--red round-full py6 px24 txt-s shadow-darken75 unselectable cursor-pointer"
+  className="btn btn--red round-full py6 px24 txt-s"
   data-test="foo"
+  disabled={false}
   onClick={[MockFunction]}
   role="button"
 >
@@ -64,6 +69,7 @@ exports[`Div styled like a medium destructive button with some extended props an
 exports[`Full width, alternate color renders as expected 1`] = `
 <button
   className="btn btn--purple round-full py12 w-full block txt-s"
+  disabled={false}
   type="button"
 >
   Here we are
@@ -88,6 +94,7 @@ exports[`Link with outline and icon text, extra padding renders as expected 1`]
 exports[`Primary renders as expected 1`] = `
 <button
   className="btn btn--blue round-full py12 px24 txt-s"
+  disabled={false}
   onClick={[MockFunction]}
   type="button"
 >
@@ -98,6 +105,7 @@ exports[`Primary renders as expected 1`] = `
 exports[`Secondary renders as expected 1`] = `
 <button
   className="btn btn--blue btn--stroke btn--stroke--2 round-full py12 px24 txt-s"
+  disabled={false}
   onClick={[MockFunction]}
   type="button"
 >
diff --git a/src/components/button/__tests__/button-test-cases.js b/src/components/button/__tests__/button-test-cases.js
index a75f11b..bee6a4e 100644
--- a/src/components/button/__tests__/button-test-cases.js
+++ b/src/components/button/__tests__/button-test-cases.js
@@ -88,19 +88,18 @@ testCases.fullWidthPurple = {
 
 testCases.weird = {
   description:
-    'Div styled like a medium destructive button with some extended props and transformed classes',
+    'Div styled like a medium destructive button with some elementAttributes',
   component: Button,
   props: {
     children: 'Save',
     size: 'medium',
     variant: 'destructive',
     onClick: safeSpy(),
-    transformClasses: classes => {
-      return classes + ' shadow-darken75 unselectable cursor-pointer';
-    },
     component: 'div',
-    role: 'button',
-    'data-test': 'foo'
+    passthroughProps: {
+      role: 'button',
+      'data-test': 'foo'
+    }
   }
 };
 
diff --git a/src/components/button/__tests__/button.test.js b/src/components/button/__tests__/button.test.js
index 975c695..0c89ba1 100644
--- a/src/components/button/__tests__/button.test.js
+++ b/src/components/button/__tests__/button.test.js
@@ -20,6 +20,13 @@ describe(testCases.primary.description, () => {
     wrapper.find('button').prop('onClick')();
     expect(testCase.props.onClick).toHaveBeenCalled();
   });
+
+  test('updates when a prop changes', () => {
+    wrapper.setProps({ color: 'pink' });
+    wrapper.update();
+
+    expect(wrapper.find('button').prop('className')).toMatch('btn--pink');
+  });
 });
 
 describe(testCases.secondary.description, () => {
diff --git a/src/components/utils/__tests__/shallow-equal-objects.test.js b/src/components/utils/__tests__/shallow-equal-objects.test.js
new file mode 100644
index 0000000..6dd5150
--- /dev/null
+++ b/src/components/utils/__tests__/shallow-equal-objects.test.js
@@ -0,0 +1,52 @@
+import shallowEqualObjects from '../shallow-equal-objects';
+
+test('returns true', () => {
+  expect(
+    shallowEqualObjects({ a: true, b: false }, { a: true, b: false })
+  ).toBe(true);
+  expect(
+    shallowEqualObjects({ a: 'true', b: 232 }, { a: 'true', b: 232 })
+  ).toBe(true);
+  const fn = () => {};
+  expect(shallowEqualObjects({ a: fn, b: 232 }, { a: fn, b: 232 })).toBe(true);
+  expect(
+    shallowEqualObjects(
+      { a: true, b: { c: false } },
+      { a: true, b: { c: false } },
+      ['b']
+    )
+  ).toBe(true);
+});
+
+test('returns false', () => {
+  expect(shallowEqualObjects({ a: true, b: false }, { a: true })).toBe(false);
+  expect(shallowEqualObjects({ a: true }, { a: false })).toBe(false);
+  expect(shallowEqualObjects({ a: 'a' }, { a: 'b' })).toBe(false);
+  expect(
+    shallowEqualObjects({ a: () => {}, b: 232 }, { a: () => {}, b: 232 })
+  ).toBe(false);
+  expect(
+    shallowEqualObjects(
+      { a: true, b: { c: false } },
+      { a: true, b: { c: true } },
+      ['b']
+    )
+  ).toBe(false);
+});
+
+test('errors', () => {
+  expect(() => {
+    shallowEqualObjects(
+      { a: true, b: { c: false } },
+      { a: true, b: { c: true } },
+      ['c']
+    );
+  }).toThrow(/can only compare objects/);
+  expect(() => {
+    shallowEqualObjects(
+      { a: true, b: { c: { d: false } } },
+      { a: true, b: { c: { d: true } } },
+      ['b']
+    );
+  }).toThrow(/can only compare objects/);
+});