Repository: couds/react-bulma-components (js)

Issue:
Warning: Failed prop type: Invalid prop `value` supplied to `Select`
Hello,

I'm experiencing a warning with `Select` when `multiple` prop is true. Can you confirm this issue?

**Bug description**

I get a warning when I use `Select` with the `multiple` prop and pass an array to `value` prop:

```
<Select multiple={true} value={[]} onChange={()=>{}}>
  <option key='1' value='1'>One</option>
  <option key='2' value='2'>Two</option>
</Select>
```

```
Warning: Failed prop type: Invalid prop `value` supplied to `Select`
```

**To Reproduce**
1. Instantiate Select with multiple={true} and value={[]}
2. See warning in developer tools console

**Expected behavior**

No warning.

**Versions**
- `react-bulma-components`: 3.4.0
- `bulma`: 0.8.2
- `react`: 17.0.1
- `Browser`: Google Chrome 87

**Additional context**

I have fixed the issue locally by changing the `value` constraint in `Select.js`:

```
value: PropTypes.oneOfType([PropTypes.string, PropTypes.number, PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.string, PropTypes.number]))]),
```

I am willing to create a pull request in the `next` branch so it can be fixed for 4.0 (I can also do a pull request for 3.4.0).

What do you think?


Pull request:
Fix Warning: Failed prop type: Invalid prop value supplied to Select
Closes #286. Please review.

Patch:
diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md
new file mode 100644
index 0000000..0007240
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/bug_report.md
@@ -0,0 +1,33 @@
+---
+name: Bug report
+about: Create a report to help us improve
+title: ''
+labels: bug
+assignees: ''
+
+---
+
+**Describe the bug**
+A clear and concise description of what the bug is.
+
+**To Reproduce**
+Steps to reproduce the behavior:
+1. Go to '...'
+2. Click on '....'
+3. Scroll down to '....'
+4. See error
+
+**Expected behavior**
+A clear and concise description of what you expected to happen.
+
+**Screenshots**
+If applicable, add screenshots to help explain your problem.
+
+**Versions**
+- `react-bulma-components:`:
+- `bulma`:
+- `react`:
+- Browser: (e.g. Firefox version 67)
+
+**Additional context**
+Add any other context about the problem here.
diff --git a/.github/ISSUE_TEMPLATE/help.md b/.github/ISSUE_TEMPLATE/help.md
new file mode 100644
index 0000000..4d7439b
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/help.md
@@ -0,0 +1,10 @@
+---
+name: Help
+about: If you need any help using this library, use this template.
+title: "[HELP]"
+labels: question
+assignees: ''
+
+---
+
+
diff --git a/src/components/button/button.js b/src/components/button/button.js
index 5dd6ca6..72557d4 100644
--- a/src/components/button/button.js
+++ b/src/components/button/button.js
@@ -17,10 +17,12 @@ const Button = ({
   size,
   outlined,
   inverted,
-  state,
   submit,
   reset,
   fullwidth,
+  focused,
+  hovered,
+  active,
   loading,
   disabled,
   remove,
@@ -31,7 +33,6 @@ const Button = ({
   text,
   ...props
 }) => {
-  // let Element = isStatic ? 'span' : renderAs;
   let otherProps = {};
   if (submit) {
     otherProps = {
@@ -63,13 +64,15 @@ const Button = ({
       className={classnames(className, {
         [`is-${color}`]: color,
         [`is-${size}`]: size,
-        [`is-${state}`]: state,
         'is-selected': isSelected,
         'is-static': isStatic,
         'is-rounded': rounded,
         'is-outlined': outlined,
         'is-inverted': inverted,
         'is-fullwidth': fullwidth,
+        'is-hovered': hovered,
+        'is-focused': focused,
+        'is-active': active,
         'is-loading': loading,
         'is-text': text,
         delete: remove,
@@ -92,11 +95,13 @@ Button.propTypes = {
   onClick: PropTypes.func,
   color: PropTypes.oneOf(colors),
   size: PropTypes.oneOf(['small', 'medium', 'large']),
-  state: PropTypes.oneOf(['hover', 'focus', 'active', 'loading']),
   outlined: PropTypes.bool,
   inverted: PropTypes.bool,
   submit: PropTypes.bool,
   reset: PropTypes.bool,
+  hovered: PropTypes.bool,
+  focused: PropTypes.bool,
+  active: PropTypes.bool,
   loading: PropTypes.bool,
   fullwidth: PropTypes.bool,
   disabled: PropTypes.bool,
@@ -116,12 +121,14 @@ Button.defaultProps = {
   onClick: () => null,
   color: undefined,
   size: undefined,
-  state: undefined,
   outlined: false,
   inverted: false,
   submit: false,
   reset: false,
   fullwidth: false,
+  hovered: false,
+  focused: false,
+  active: false,
   loading: false,
   disabled: false,
   remove: false,
diff --git a/src/components/form/components/select.js b/src/components/form/components/select.js
index 58a0358..f070f5b 100644
--- a/src/components/form/components/select.js
+++ b/src/components/form/components/select.js
@@ -21,30 +21,39 @@ const Select = ({
   name,
   domRef,
   ...props
-}) => (
-  <Element
-    domRef={domRef}
-    className={classnames('select', className, {
-      [`is-${size}`]: size,
-      [`is-${color}`]: color,
-      'is-loading': loading,
-      'is-multiple': multiple,
-    })}
-    style={style}
-  >
+}) => {
+  /**
+   * Return default value for value prop
+   */
+  function defaultValue() {
+    return multiple ? [] : '';
+  }
+
+  return (
     <Element
-      renderAs="select"
-      {...props}
-      multiple={multiple}
-      value={value}
-      readOnly={readOnly}
-      disabled={disabled}
-      name={name}
+      domRef={domRef}
+      className={classnames('select', className, {
+        [`is-${size}`]: size,
+        [`is-${color}`]: color,
+        'is-loading': loading,
+        'is-multiple': multiple,
+      })}
+      style={style}
     >
-      {children}
+      <Element
+        renderAs="select"
+        {...props}
+        multiple={multiple}
+        value={value !== undefined ? value : defaultValue()}
+        readOnly={readOnly}
+        disabled={disabled}
+        name={name}
+      >
+        {children}
+      </Element>
     </Element>
-  </Element>
-);
+  );
+};
 
 Select.propTypes = {
   ...modifiers.propTypes,
@@ -57,13 +66,29 @@ Select.propTypes = {
   disabled: PropTypes.bool,
   multiple: PropTypes.bool,
   loading: PropTypes.bool,
-  value: PropTypes.oneOfType([
-    PropTypes.string,
-    PropTypes.number,
-    PropTypes.arrayOf(
-      PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
-    ),
-  ]),
+  value: function (props, propName, componentName) {
+    if (props.multiple) {
+      PropTypes.checkPropTypes(
+        {
+          [propName]: PropTypes.arrayOf(
+            PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
+          ),
+        },
+        props,
+        propName,
+        componentName,
+      );
+    } else {
+      PropTypes.checkPropTypes(
+        {
+          [propName]: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
+        },
+        props,
+        propName,
+        componentName,
+      );
+    }
+  },
   /**
    * The name of the input field Commonly used for [multi-input handling](https://reactjs.org/docs/forms.html#handling-multiple-inputs)
    */
@@ -74,7 +99,7 @@ Select.defaultProps = {
   ...modifiers.defaultProps,
   children: null,
   className: undefined,
-  value: '',
+  value: undefined,
   style: undefined,
   size: undefined,
   color: undefined,
diff --git a/src/components/navbar/navbar.story.js b/src/components/navbar/navbar.story.js
index 2f90594..c6ce4cb 100644
--- a/src/components/navbar/navbar.story.js
+++ b/src/components/navbar/navbar.story.js
@@ -11,7 +11,7 @@ storiesOf('Navbar', module)
     <div>
       {story()}
       <Box style={{ margin: '15px auto', maxWidth: '75vw' }}>
-        Check the Know tab to see the behavior of this component
+        Check the Knobs tab to see the behavior of this component
       </Box>
     </div>
   ))


Test patch:
diff --git a/src/components/button/__test__/__snapshots__/button.test.js.snap b/src/components/button/__test__/__snapshots__/button.test.js.snap
index 0cbf599..77171cf 100644
--- a/src/components/button/__test__/__snapshots__/button.test.js.snap
+++ b/src/components/button/__test__/__snapshots__/button.test.js.snap
@@ -176,6 +176,15 @@ exports[`Button component Should exist 1`] = `[Function]`;
 
 exports[`Button component Should expose Button Group 1`] = `[Function]`;
 
+exports[`Button component Should render active state 1`] = `
+<button
+  className="is-active button"
+  disabled={false}
+  onClick={[Function]}
+  tabIndex={0}
+/>
+`;
+
 exports[`Button component Should render as a React element link with to prop 1`] = `
 <a
   href="http://google.com"
@@ -209,3 +218,30 @@ exports[`Button component Should render be disabled 1`] = `
   tabIndex={-1}
 />
 `;
+
+exports[`Button component Should render focused state 1`] = `
+<button
+  className="is-focused button"
+  disabled={false}
+  onClick={[Function]}
+  tabIndex={0}
+/>
+`;
+
+exports[`Button component Should render hovered state 1`] = `
+<button
+  className="is-hovered button"
+  disabled={false}
+  onClick={[Function]}
+  tabIndex={0}
+/>
+`;
+
+exports[`Button component Should render loading state 1`] = `
+<button
+  className="is-loading button"
+  disabled={false}
+  onClick={[Function]}
+  tabIndex={0}
+/>
+`;
diff --git a/src/components/button/__test__/button.test.js b/src/components/button/__test__/button.test.js
index 7f95391..8a02e52 100644
--- a/src/components/button/__test__/button.test.js
+++ b/src/components/button/__test__/button.test.js
@@ -61,6 +61,22 @@ describe('Button component', () => {
     const component = renderer.create(<Button disabled />);
     expect(component.toJSON()).toMatchSnapshot();
   });
+  it('Should render loading state', () => {
+    const component = renderer.create(<Button loading />);
+    expect(component.toJSON()).toMatchSnapshot();
+  });
+  it('Should render focused state', () => {
+    const component = renderer.create(<Button focused />);
+    expect(component.toJSON()).toMatchSnapshot();
+  });
+  it('Should render hovered state', () => {
+    const component = renderer.create(<Button hovered />);
+    expect(component.toJSON()).toMatchSnapshot();
+  });
+  it('Should render active state', () => {
+    const component = renderer.create(<Button active />);
+    expect(component.toJSON()).toMatchSnapshot();
+  });
   it('Should be a submit form button', () => {
     const component = renderer.create(<Button submit />);
     expect(component.toJSON()).toMatchSnapshot();
diff --git a/src/components/form/components/__test__/__snapshots__/select.test.js.snap b/src/components/form/components/__test__/__snapshots__/select.test.js.snap
index ec78537..1e02077 100644
--- a/src/components/form/components/__test__/__snapshots__/select.test.js.snap
+++ b/src/components/form/components/__test__/__snapshots__/select.test.js.snap
@@ -8,7 +8,7 @@ exports[`Select component Should be large, red, disabled and multioption 1`] = `
     disabled={true}
     multiple={true}
     readOnly={false}
-    value=""
+    value={Array []}
   >
     <option>
       1
@@ -98,3 +98,35 @@ exports[`Select component Should use inline styles 1`] = `
   </select>
 </div>
 `;
+
+exports[`Select component value PropTypes Should be multioption and should not accept non array value 1`] = `
+<div
+  className="select is-multiple"
+>
+  <select
+    disabled={false}
+    multiple={true}
+    readOnly={false}
+    value={1}
+  />
+</div>
+`;
+
+exports[`Select component value PropTypes Should not be multioption and should not accept array value 1`] = `
+<div
+  className="select"
+>
+  <select
+    disabled={false}
+    multiple={false}
+    readOnly={false}
+    value={
+      Array [
+        1,
+        2,
+        3,
+      ]
+    }
+  />
+</div>
+`;
diff --git a/src/components/form/components/__test__/select.test.js b/src/components/form/components/__test__/select.test.js
index 1631791..33fa46c 100644
--- a/src/components/form/components/__test__/select.test.js
+++ b/src/components/form/components/__test__/select.test.js
@@ -46,4 +46,20 @@ describe('Select component', () => {
     );
     expect(component.toJSON()).toMatchSnapshot();
   });
+  describe('value PropTypes', () => {
+    it('Should be multioption and should not accept non array value', () => {
+      const spy = jest.spyOn(console, 'error').mockImplementation();
+      const component = renderer.create(<Select multiple value={1}></Select>);
+      expect(component.toJSON()).toMatchSnapshot();
+      expect(spy).toBeCalled();
+      spy.mockRestore();
+    });
+    it('Should not be multioption and should not accept array value', () => {
+      const spy = jest.spyOn(console, 'error').mockImplementation();
+      const component = renderer.create(<Select value={[1, 2, 3]}></Select>);
+      expect(component.toJSON()).toMatchSnapshot();
+      expect(spy).toBeCalled();
+      spy.mockRestore();
+    });
+  });
 });