Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Component/tri state switch #575

Merged
merged 3 commits into from
Feb 12, 2018
Merged

Component/tri state switch #575

merged 3 commits into from
Feb 12, 2018

Conversation

kristofferjs
Copy link
Contributor

Shows that a boolean is undefined. Usually shows when creating a new document in studio.
There is no support for setting the checkbox/switch back to undefined at this point.

Copy link
Member

@rexxars rexxars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good apart from the stray console.log :)

@@ -32,13 +32,15 @@ export default class BooleanInput extends React.Component<Props> {
render() {
const {value, type, level, description, ...rest} = this.props

console.log('boolean', value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stray console.log

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -36,6 +36,10 @@
composes: root;
}

.undefinedChecked {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to call it .indeterminate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -38,19 +38,29 @@ export default class Checkbox extends React.Component {

setInput = el => {
this._input = el

if (typeof value === 'undefined' && el) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be moved to componentDidMount instead? Personally that is the first place I'd look for it and expect it to be. And you don't have to worry about el (this._input) being null either.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This applies to Switch as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@kristofferjs kristofferjs merged commit f2ae92d into next Feb 12, 2018
@kristofferjs kristofferjs deleted the component/tri-state-switch branch February 12, 2018 06:39
kristofferjs pushed a commit that referenced this pull request Feb 12, 2018
* [components] Tri state switch

* [components] Support indeterminate state on checkboxes #47

* [components] Changes from PR
kristofferjs pushed a commit that referenced this pull request Feb 14, 2018
* [components] Tri state switch

* [components] Support indeterminate state on checkboxes #47

* [components] Changes from PR
kristofferjs pushed a commit that referenced this pull request Feb 15, 2018
* [components] Tri state switch

* [components] Support indeterminate state on checkboxes #47

* [components] Changes from PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants