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

added checkbox option for BoolField for antd #348

Merged
merged 2 commits into from
Sep 26, 2017

Conversation

purplecones
Copy link
Contributor

@purplecones purplecones commented Sep 26, 2017

PR adds ability to create an antd Checkbox with uniforms BoolField instead of just the antd Switch.

Usage

<BoolField
    checkbox
    name="myCheckbox"
    label="Check here if you agree"
/>


@codecov
Copy link

codecov bot commented Sep 26, 2017

Codecov Report

Merging #348 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #348   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files         156    156           
  Lines        1305   1305           
=====================================
  Hits         1305   1305
Impacted Files Coverage Δ
packages/uniforms-antd/src/BoolField.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 313344b...f9a9cbc. Read the comment docs.

Copy link
Contributor

@radekmie radekmie left a comment

Choose a reason for hiding this comment

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

Thank you very much, especially for adding tests!

@@ -1,83 +1,155 @@
import React from 'react';
import Switch from 'antd/lib/switch';
import Checkbox from 'antd/lib/checkbox';
import {mount} from 'enzyme';
Copy link
Contributor

Choose a reason for hiding this comment

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

These should be aligned.

@@ -1,26 +1,38 @@
import Icon from 'antd/lib/icon';
import React from 'react';
import Switch from 'antd/lib/switch';
import Checkbox from 'antd/lib/checkbox';
import connectField from 'uniforms/connectField';
import filterDOMProps from 'uniforms/filterDOMProps';
Copy link
Contributor

Choose a reason for hiding this comment

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

These should stay in order.

onChange={() => props.onChange(!props.value)}
ref={props.inputRef}
{...filterDOMProps(props)}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

How about React.createElement(props.checkbox ? Checkbox : Switch, {...})? It'll reduce this duplicated code.

@@ -2,6 +2,7 @@ const unwantedProps = [
// These props are provided by BaseField
'changed',
'changedMap',
'checkbox',
Copy link
Contributor

Choose a reason for hiding this comment

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

These are global, so it's a breaking change. I'd rather go with destructuring props in BoolField - it won't be needed then.

@radekmie radekmie added the Type: Feature New features and feature requests label Sep 26, 2017
@purplecones
Copy link
Contributor Author

Thanks for feedback, learning something new every day.

@radekmie radekmie merged commit d7667c7 into vazco:master Sep 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New features and feature requests
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants