feat(component): rename onChange to onItemChange (#251)

BREAKING CHANGE: `onChange` is renamed to `onItemChange`
bigcommerce · Nov 11, 2019 · 7e609d8 · 7e609d8
1 parent a1d41df
commit 7e609d8
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 41 deletions.
diff --git a/packages/big-design/src/components/Select/Select.tsx b/packages/big-design/src/components/Select/Select.tsx
@@ -70,7 +70,7 @@ export class Select<T extends any> extends React.PureComponent<SelectProps<T>, S
       label,
       maxHeight,
       multi,
-      onChange,
+      onItemChange,
       placeholder,
       placement,
       value,
@@ -132,7 +132,17 @@ export class Select<T extends any> extends React.PureComponent<SelectProps<T>, S
   }
 
   private renderInput() {
-    const { name, placeholder, error, filterable = true, required, disabled, onChange, options, value } = this.props;
+    const {
+      name,
+      placeholder,
+      error,
+      filterable = true,
+      required,
+      disabled,
+      onItemChange,
+      options,
+      value,
+    } = this.props;
     const { highlightedItem, inputText, isOpen } = this.state;
     const ariaActiveDescendant = highlightedItem ? { 'aria-activedescendant': highlightedItem.id } : {};
     const ariaControls = isOpen ? { 'aria-controls': this.getSelectId() } : {};
@@ -149,7 +159,7 @@ export class Select<T extends any> extends React.PureComponent<SelectProps<T>, S
           })
         : [];
 
-      onChange(filteredValues, this.getSelectedOptions(filteredValues));
+      onItemChange(filteredValues, this.getSelectedOptions(filteredValues));
       this.focusInput();
     };
 
@@ -504,7 +514,7 @@ export class Select<T extends any> extends React.PureComponent<SelectProps<T>, S
   };
 
   private handleOnCheckboxOptionClick = (option: Option<T>) => {
-    const { onChange, value } = this.props;
+    const { onItemChange, value } = this.props;
     const { highlightedItem } = this.state;
     let updatedValues = [];
 
@@ -520,18 +530,18 @@ export class Select<T extends any> extends React.PureComponent<SelectProps<T>, S
       updatedValues = value.concat(option.value);
     }
 
-    onChange(updatedValues, this.getSelectedOptions(updatedValues));
+    onItemChange(updatedValues, this.getSelectedOptions(updatedValues));
     this.focusInput();
   };
 
   private handleOnOptionClick = (option: Option<T>) => {
-    const { onChange } = this.props;
+    const { onItemChange } = this.props;
 
     if (option.disabled) {
       return;
     }
 
-    onChange(option.value, option);
+    onItemChange(option.value, option);
     this.toggleList();
   };
 
@@ -637,7 +647,7 @@ export class Select<T extends any> extends React.PureComponent<SelectProps<T>, S
           if (Array.isArray(this.props.value)) {
             const updatedValues = this.props.value.slice(0, this.props.value.length - 1);
 
-            this.props.onChange(updatedValues, this.getSelectedOptions(updatedValues));
+            this.props.onItemChange(updatedValues, this.getSelectedOptions(updatedValues));
           }
         }
         break;

diff --git a/packages/big-design/src/components/Select/spec.tsx b/packages/big-design/src/components/Select/spec.tsx
@@ -7,7 +7,7 @@ import { Form } from '../Form';
 
 import { Select } from './Select';
 
-const onChange = jest.fn();
+const onItemChange = jest.fn();
 const onActionClick = jest.fn();
 
 const mockOptions = [
@@ -27,7 +27,7 @@ const SelectMock = (
       onClick: onActionClick,
     }}
     error="Required"
-    onChange={onChange}
+    onItemChange={onItemChange}
     label="Countries"
     placeholder="Choose country"
     options={mockOptions}
@@ -38,7 +38,7 @@ const SelectMock = (
 
 const MultiselectMock = (
   <Select
-    onChange={onChange}
+    onItemChange={onItemChange}
     label="Countries"
     multi
     placeholder="Choose country"
@@ -261,17 +261,17 @@ test('end should select last select item', () => {
   expect(input.getAttribute('aria-activedescendant')).toEqual(options[5].id);
 });
 
-test('enter should trigger onChange', () => {
+test('enter should trigger onItemChange', () => {
   const { getAllByLabelText } = render(SelectMock);
   const input = getAllByLabelText('Countries')[0];
 
   fireEvent.focus(input);
   fireEvent.keyDown(input, { key: 'ArrowDown' });
   fireEvent.keyDown(input, { key: 'Enter' });
-  expect(onChange).toHaveBeenCalledWith(mockOptions[2].value, mockOptions[2]);
+  expect(onItemChange).toHaveBeenCalledWith(mockOptions[2].value, mockOptions[2]);
 });
 
-test('clicking on select options should trigger onChange', () => {
+test('clicking on select options should trigger onItemChange', () => {
   const { getAllByRole, getAllByLabelText } = render(SelectMock);
   const input = getAllByLabelText('Countries')[0];
 
@@ -281,7 +281,7 @@ test('clicking on select options should trigger onChange', () => {
 
   fireEvent.mouseOver(options[1]);
   fireEvent.click(options[1]);
-  expect(onChange).toHaveBeenCalledWith(mockOptions[1].value, mockOptions[1]);
+  expect(onItemChange).toHaveBeenCalledWith(mockOptions[1].value, mockOptions[1]);
 });
 
 test('clicking on disabled select options should not trigger onClick', () => {
@@ -362,7 +362,7 @@ test('select should render an error if one is provided', () => {
   const { getByText } = render(
     <Form.Group>
       <Select
-        onChange={onChange}
+        onItemChange={onItemChange}
         label="Countries"
         error="Required"
         placeholder="Choose country"
@@ -392,7 +392,7 @@ test('select should have a required attr if set as required', () => {
 test('select should not have a required attr if not set as required', () => {
   const { getAllByLabelText } = render(
     <Select
-      onChange={onChange}
+      onItemChange={onItemChange}
       label="Countries"
       placeholder="Choose country"
       options={[
@@ -414,7 +414,7 @@ test('select should have a disabled attr if set as disabled', () => {
   const { getAllByLabelText } = render(
     <Select
       disabled
-      onChange={onChange}
+      onItemChange={onItemChange}
       label="Countries"
       placeholder="Choose country"
       options={[
@@ -471,7 +471,7 @@ test('multiselect should be able to select multiple options', () => {
   fireEvent.keyDown(input, { key: 'ArrowDown' });
   fireEvent.keyDown(input, { key: 'Enter' });
 
-  expect(onChange).toHaveBeenCalledWith(
+  expect(onItemChange).toHaveBeenCalledWith(
     [mockOptions[0].value, mockOptions[1].value, mockOptions[2].value],
     [mockOptions[0], mockOptions[1], mockOptions[2]],
   );
@@ -486,7 +486,7 @@ test('multiselect should be able to deselect options', () => {
   fireEvent.keyDown(input, { key: 'ArrowDown' });
   fireEvent.keyDown(input, { key: 'Enter' });
 
-  expect(onChange).toHaveBeenCalledWith([mockOptions[1].value], [mockOptions[1]]);
+  expect(onItemChange).toHaveBeenCalledWith([mockOptions[1].value], [mockOptions[1]]);
 });
 
 test('chips should be rendered', () => {
@@ -500,7 +500,7 @@ test('chips should be rendered', () => {
 test('appends (optional) text to label if select is not required', () => {
   const { container } = render(
     <Select
-      onChange={onChange}
+      onItemChange={onItemChange}
       label="Countries"
       options={[
         { value: 'us', content: 'United States' },
@@ -521,7 +521,7 @@ test('does not forward styles', () => {
   const { container, getByRole } = render(
     <Select
       className="test"
-      onChange={onChange}
+      onItemChange={onItemChange}
       label="Countries"
       options={[
         { value: 'us', content: 'United States' },
@@ -544,7 +544,7 @@ test('should render a non filterable select', () => {
     <Select
       filterable={false}
       label="Countries"
-      onChange={onChange}
+      onItemChange={onItemChange}
       options={[
         { value: 'us', content: 'United States' },
         { value: 'mx', content: 'Mexico' },
@@ -566,7 +566,7 @@ test('should use the passed in ref object if provided', () => {
     <Select
       inputRef={ref}
       label="Countries"
-      onChange={onChange}
+      onItemChange={onItemChange}
       options={[
         { value: 'us', content: 'United States' },
         { value: 'mx', content: 'Mexico' },
@@ -590,7 +590,7 @@ test('should call the provided refSetter if any', () => {
     <Select
       inputRef={refSetter}
       label="Countries"
-      onChange={onChange}
+      onItemChange={onItemChange}
       options={[
         { value: 'us', content: 'United States' },
         { value: 'mx', content: 'Mexico' },

diff --git a/packages/big-design/src/components/Select/types.ts b/packages/big-design/src/components/Select/types.ts
@@ -18,7 +18,7 @@ export interface SelectProps<T> extends Omit<React.HTMLAttributes<HTMLUListEleme
   positionFixed?: boolean;
   required?: boolean;
   value?: T | T[];
-  onChange(value: T | T[], option: Option<T> | Array<Option<T>>): void;
+  onItemChange(value: T | T[], option: Option<T> | Array<Option<T>>): void;
 }
 
 interface BaseItem extends Omit<ListItemProps, 'children' | 'content' | 'value'> {

diff --git a/packages/docs/PropTables/SelectPropTable.tsx b/packages/docs/PropTables/SelectPropTable.tsx
@@ -113,7 +113,7 @@ const selectProps: Prop[] = [
   },
 
   {
-    name: 'onChange',
+    name: 'onItemChange',
     types: '(value: string | number | Array<string | number>) => void',
     required: true,
     description: 'Callback called with value of selected option.',

diff --git a/packages/docs/pages/Form/FormPage.tsx b/packages/docs/pages/Form/FormPage.tsx
@@ -86,7 +86,7 @@ export default () => (
         <Form.Group>
           <Select
             label="Example Select"
-            onChange={() => null}
+            onItemChange={() => null}
             options={[
               { value: 1, content: 'Option' },
               { value: 2, content: 'Option' },

diff --git a/packages/docs/pages/Select/SelectPage.tsx b/packages/docs/pages/Select/SelectPage.tsx
@@ -35,7 +35,7 @@ export default () => (
               filterable={true}
               label="Countries"
               maxHeight={300}
-              onChange={handleChange}
+              onItemChange={handleChange}
               options={[
                 { value: 'us', content: 'United States' },
                 { value: 'mx', content: 'Mexico' },
@@ -88,7 +88,7 @@ export default () => (
               label="States"
               maxHeight={300}
               multi={true}
-              onChange={handleChange}
+              onItemChange={handleChange}
               options={[
                 { value: 'tx', content: 'Texas' },
                 { value: 'ca', content: 'California' },
@@ -119,7 +119,7 @@ export default () => (
       <Grid gridColumns="repeat(4, 1fr)">
         <Select
           label="Select"
-          onChange={() => null}
+          onItemChange={() => null}
           options={[
             { value: 1, content: 'Option' },
             { value: 2, content: 'Option' },
@@ -132,7 +132,7 @@ export default () => (
         />
         <Select
           label="Select"
-          onChange={() => null}
+          onItemChange={() => null}
           options={[
             { value: 1, content: 'Option' },
             { value: 2, content: 'Option' },
@@ -145,7 +145,7 @@ export default () => (
         />
         <Select
           label="Select"
-          onChange={() => null}
+          onItemChange={() => null}
           options={[
             { value: 1, content: 'Option' },
             { value: 2, content: 'Option' },
@@ -158,7 +158,7 @@ export default () => (
         />
         <Select
           label="Select"
-          onChange={() => null}
+          onItemChange={() => null}
           options={[
             { value: 1, content: 'Option' },
             { value: 2, content: 'Option' },
@@ -185,7 +185,7 @@ export default () => (
       <Grid gridColumns="repeat(3, 1fr)">
         <Select
           label="Select"
-          onChange={() => null}
+          onItemChange={() => null}
           options={[
             { value: 1, content: 'Option' },
             { value: 2, content: 'Option' },
@@ -197,7 +197,7 @@ export default () => (
         />
         <Select
           label="Select"
-          onChange={() => null}
+          onItemChange={() => null}
           options={[
             { value: 1, content: 'Option' },
             { value: 2, content: 'Option' },
@@ -210,7 +210,7 @@ export default () => (
         <Select
           label="Select"
           maxHeight={150}
-          onChange={() => null}
+          onItemChange={() => null}
           options={[
             { value: 1, content: 'Option' },
             { value: 2, content: 'Option' },
@@ -223,7 +223,7 @@ export default () => (
         <Select
           label="Select"
           maxHeight={350}
-          onChange={() => null}
+          onItemChange={() => null}
           options={[
             { value: 1, content: 'Option' },
             { value: 2, content: 'Option' },
@@ -250,7 +250,7 @@ export default () => (
         disabled
         label="Select"
         maxHeight={350}
-        onChange={() => null}
+        onItemChange={() => null}
         options={[
           { value: 1, content: 'Option' },
           { value: 2, content: 'Option' },
@@ -279,7 +279,7 @@ export default () => (
           onClick: () => null,
         }}
         label="Countries"
-        onChange={() => null}
+        onItemChange={() => null}
         options={[
           { value: 'us', content: 'United States' },
           { value: 'mx', content: 'Mexico' },
@@ -305,7 +305,7 @@ export default () => (
         <Select
           label="Countries"
           error="Need to choose a country before proceeding"
-          onChange={() => null}
+          onItemChange={() => null}
           options={[
             { value: 'us', content: 'United States' },
             { value: 'mx', content: 'Mexico' },