Skip to content

[Select] Set type undefined rather than null#10430

Merged
oliviertassinari merged 1 commit into
mui:v1-betafrom
caub:type-undef
Feb 24, 2018
Merged

[Select] Set type undefined rather than null#10430
oliviertassinari merged 1 commit into
mui:v1-betafrom
caub:type-undef

Conversation

@caub
Copy link
Copy Markdown
Contributor

@caub caub commented Feb 24, 2018

continuation of #10361 (comment)

I couldn't use defaultProps for type,
If I put a

SelectInput.defaultProps = {
  type: undefined,
};

and remove = 'hidden' in render

test:unit fails with:

1) <Select> integration
       with Dialog
         should focus the selected item:
     Error: Uncaught [Error: Warning: Failed prop type: You provided a `value` prop to a form field without an `onChange` handler. This will render a read-only field. If the field should be mutable use `defaultValue`. Otherwise, set either `onChange` or `readOnly`

note: would you like to refactor all components, and define propTypes and defaultProps as static properties at the start of component class?
If you want them at the end, still possible too:

class SelectInput extends React.Component {
  static propTypes = propTypes;
  state = {
    open: false,
  };
  // ...

} 

const propTypes = {
  //..
};

@oliviertassinari
Copy link
Copy Markdown
Member

Interesting, what do we win from this change?

@caub
Copy link
Copy Markdown
Contributor Author

caub commented Feb 24, 2018

not much :), it allows to force it to null, and I find destructuring defaults clearer personally. But if you do <Select type={null} it will stay as null

@oliviertassinari oliviertassinari changed the title [Select] set type undefined rather than null [Select] Set type undefined rather than null Feb 24, 2018
@oliviertassinari oliviertassinari merged commit be50ba9 into mui:v1-beta Feb 24, 2018
@oliviertassinari
Copy link
Copy Markdown
Member

@caub Alright, let's try that :)

@oliviertassinari oliviertassinari added the type: new feature Expand the scope of the product to solve a new problem. label Feb 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: select Changes related to the select. type: new feature Expand the scope of the product to solve a new problem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants