feat(Checkbox): add component by layershifter · Pull Request #1405 · microsoft/fluent-ui-react

layershifter · 2019-05-28T15:47:02Z

Fixes #634.

This PR adds a base implementation of Checkbox component with basiccheckboxBehavior. Also adds stardust-checkmark icon that is used for Checkbox component.

`<Checkbox checked />`

`<Checkbox checked toogle />`

Playground

…rdust-ui/react into feat/checkbox

codecov · 2019-05-30T13:23:59Z

Codecov Report

Merging #1405 into master will decrease coverage by 0.35%.
The diff coverage is 13.51%.

@@            Coverage Diff             @@
##           master    #1405      +/-   ##
==========================================
- Coverage   73.59%   73.23%   -0.36%     
==========================================
  Files         787      794       +7     
  Lines        5888     5923      +35     
  Branches     1737     1726      -11     
==========================================
+ Hits         4333     4338       +5     
- Misses       1549     1579      +30     
  Partials        6        6

Impacted Files	Coverage Δ
packages/react/src/index.ts	`100% <ø> (ø)`	⬆️
packages/react/src/themes/teams/index.tsx	`90.9% <ø> (ø)`	⬆️
...act/src/themes/teams/components/Icon/iconStyles.ts	`16.12% <ø> (+0.97%)`	⬆️
...react/src/themes/base/components/Icon/iconNames.ts	`75% <ø> (ø)`	⬆️
...emes/base/components/Checkbox/checkboxVariables.ts	`0% <0%> (ø)`
...mes/teams/components/Checkbox/checkboxVariables.ts	`0% <0%> (ø)`
.../themes/base/components/Checkbox/checkboxStyles.ts	`10% <10%> (ø)`
...ms/components/Icon/svg/icons/stardustCheckmark.tsx	`100% <100%> (ø)`
...cessibility/Behaviors/Checkbox/checkboxBehavior.ts	`100% <100%> (ø)`
...ackages/react/src/components/Checkbox/Checkbox.tsx	`4.76% <4.76%> (ø)`
... and 8 more

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 bdade88...3e8727b. Read the comment docs.

codecov · 2019-05-30T13:24:00Z

Codecov Report

Merging #1405 into master will decrease coverage by 0.11%.
The diff coverage is 57.14%.

@@            Coverage Diff             @@
##           master    #1405      +/-   ##
==========================================
- Coverage    73.6%   73.49%   -0.12%     
==========================================
  Files         787      793       +6     
  Lines        5907     5949      +42     
  Branches     1744     1735       -9     
==========================================
+ Hits         4348     4372      +24     
- Misses       1553     1571      +18     
  Partials        6        6

Impacted Files	Coverage Δ
packages/react/src/index.ts	`100% <ø> (ø)`	⬆️
packages/react/src/themes/teams/index.tsx	`90.9% <ø> (ø)`	⬆️
...react/src/themes/base/components/Icon/iconNames.ts	`75% <ø> (ø)`	⬆️
...emes/base/components/Checkbox/checkboxVariables.ts	`0% <0%> (ø)`
...mes/teams/components/Checkbox/checkboxVariables.ts	`0% <0%> (ø)`
.../themes/base/components/Checkbox/checkboxStyles.ts	`10% <10%> (ø)`
...cessibility/Behaviors/Checkbox/checkboxBehavior.ts	`100% <100%> (ø)`
...themes/teams/components/Checkbox/checkboxStyles.ts	`50% <50%> (ø)`
...ackages/react/src/components/Checkbox/Checkbox.tsx	`77.77% <77.77%> (ø)`
... and 5 more

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 882f293...75b62ed. Read the comment docs.

mnajdova · 2019-05-30T16:58:58Z

packages/react/src/themes/base/components/Checkbox/checkboxVariables.ts

+
+export default (siteVars: any): CheckboxVariables => ({
+  checkboxBackground: 'transparent',
+  checkboxBorderColor: siteVars.colors.grey[300],


Let's try to use the color scheme here, as this is a fresh new component ;)

I meant in teams theme :)

As we decided not to add the color scheme now and make the component red-lined, let's be sure to track this work

packages/react/src/themes/teams/index.tsx

…rdust-ui/react into feat/checkbox # Conflicts: # packages/react/src/themes/teams/components/Icon/iconStyles.ts

docs/src/components/ExampleSnippet/renderElementToJSX.ts

mnajdova · 2019-06-03T10:10:37Z

packages/react/src/components/Checkbox/Checkbox.tsx

+
+  handleClick = (e: React.MouseEvent | React.KeyboardEvent) => {
+    const { disabled } = this.props
+    const checked = !this.state.checked


nit, could be moved in the if statement

Will keep as it is for now :)

mnajdova · 2019-06-03T10:17:21Z

packages/react/src/components/Checkbox/Checkbox.tsx

+import { Accessibility } from '../../lib/accessibility/types'
+import { checkboxBehavior } from '../../lib/accessibility'
+
+export interface CheckboxProps extends UIComponentProps, ChildrenComponentProps {


Let's add conformat test for the new component.

Added, have to add an additional handler for onChange. Decided to keep it flat to be more obvious

mnajdova · 2019-06-03T10:20:17Z

packages/react/src/lib/accessibility/Behaviors/Checkbox/checkboxBehavior.ts

+    root: {
+      'aria-checked': props.checked,
+      role: 'checkbox',
+      tabIndex: 0,


Just to be sure, we should not add any attribute if the checkbox is disabled?

https://www.w3.org/TR/wai-aria-practices/examples/checkbox/checkbox-1/checkbox-1.html

I don't see this in the spec, so decided to skip for the initial implementation

packages/react/src/themes/teams/components/Checkbox/checkboxStyles.ts

packages/react/src/themes/base/components/Checkbox/checkboxVariables.ts

mnajdova · 2019-06-03T13:36:44Z

packages/react/src/components/Checkbox/Checkbox.tsx

+        className={classes.root}
+        onClick={this.handleClick}
+        onChange={this.handleChange}
+        onFocus={this.handleFocus}


Was this the thing missing for the focus styles? :)

Yep :) No IDE highlight 😭

layershifter force-pushed the feat/checkbox branch 2 times, most recently from b4f14b1 to c9e09f8 Compare May 29, 2019 14:33

feat(Checkbox): add component

72a2e76

layershifter force-pushed the feat/checkbox branch from c9e09f8 to 72a2e76 Compare May 29, 2019 14:41

layershifter added 5 commits May 29, 2019 17:00

add checkmark icon

e459ebb

Merge branches 'feat/checkbox' and 'master' of https://github.com/sta…

209167a

…rdust-ui/react into feat/checkbox

add behaviour import

6af18cb

rename files

e78f34a

revert change in example

3e8727b

layershifter changed the title ~~[WIP] feat(Checkbox): add component~~ feat(Checkbox): add component May 30, 2019

Merge branches 'feat/checkbox' and 'master' of https://github.com/sta…

4116963

…rdust-ui/react into feat/checkbox

layershifter added ⚙️ enhancement New feature or request 🚀 ready for review labels May 30, 2019

layershifter marked this pull request as ready for review May 30, 2019 13:21

layershifter requested review from dvdzkwsk, kuzhelov, levithomason, miroslavstastny and mnajdova as code owners May 30, 2019 13:21

mnajdova reviewed May 30, 2019

View reviewed changes

packages/react/src/themes/teams/index.tsx Outdated Show resolved Hide resolved

layershifter added 3 commits May 31, 2019 14:28

remove custom icon, use accept

a99363e

Merge branches 'feat/checkbox' and 'master' of https://github.com/sta…

3ce9692

…rdust-ui/react into feat/checkbox # Conflicts: # packages/react/src/themes/teams/components/Icon/iconStyles.ts

add changelog entry

489db2c

mnajdova reviewed Jun 3, 2019

View reviewed changes

docs/src/components/ExampleSnippet/renderElementToJSX.ts Show resolved Hide resolved

mnajdova reviewed Jun 3, 2019

View reviewed changes

packages/react/src/themes/teams/components/Checkbox/checkboxStyles.ts Show resolved Hide resolved

mnajdova reviewed Jun 3, 2019

View reviewed changes

packages/react/src/themes/base/components/Checkbox/checkboxVariables.ts Outdated Show resolved Hide resolved

layershifter added 3 commits June 3, 2019 15:11

and conformance test and onChange handler

3d13c72

add onFocus handler

b57a98b

update variables

75b62ed

mnajdova reviewed Jun 3, 2019

View reviewed changes

mnajdova approved these changes Jun 3, 2019

View reviewed changes

layershifter merged commit dfd26e5 into master Jun 3, 2019

delete-merged-branch bot deleted the feat/checkbox branch June 3, 2019 13:59

jurokapsiar mentioned this pull request Aug 5, 2019

Add support for input slider and toggle button #1255

Closed

Conversation

layershifter commented May 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

<Checkbox checked />

<Checkbox checked toogle />

Playground

Uh oh!

codecov bot commented May 30, 2019

Codecov Report

Uh oh!

codecov bot commented May 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mnajdova May 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

layershifter commented May 28, 2019 •

edited

Loading

`<Checkbox checked />`

`<Checkbox checked toogle />`

codecov bot commented May 30, 2019 •

edited

Loading

mnajdova May 30, 2019 •

edited

Loading