fix(Checkbox): match DOM checkbox behavior #2748

levithomason · 2018-04-28T20:51:09Z

A controlled Checkbox did not fire onClick / onChange when pressing spacebar after focus. This was strictly when a functional state updater was used.

Changes

I've commented on relevant changes to help separate the prettier updates from the actual code changes.

Change Checkbox handlers so that mouse down, mouse up, click, and change events are identical to the DOM checkbox
Add example comparing a vanilla DOM checkbox to our Checkbox with an event log
Rearrange "Remote" checkbox example from "States" to "Usage" and rename "Controlled"

TODO

Help much appreciated! Make PRs against fix/checkbox-spacebar instead of master.

Add/fix tests (Note, the new example works but the tests don't seem to think so.)

levithomason · 2018-04-28T20:55:40Z

src/modules/Checkbox/Checkbox.js

      >
        <input
          {...htmlInputProps}
          checked={checked}
          className='hidden'
          id={id}
          name={name}
-          onClick={this.handleInputClick}
+          onChange={this.handleInputClick}


Relevant change, remove duplicate click firing.

levithomason · 2018-04-28T20:55:44Z

src/modules/Checkbox/Checkbox.js

        onChange={this.handleContainerClick}
        onMouseDown={this.handleMouseDown}
+        onMouseUp={this.handleMouseUp}


Relevant change

levithomason · 2018-04-28T20:55:59Z

src/modules/Checkbox/Checkbox.js

+    _.invoke(this.props, 'onMouseUp', e, { ...this.props, checked: !!checked, indeterminate: !!indeterminate })
+
+    this.handleClick(e)
+  }


Relevant change, click is fired on mouse up.

levithomason · 2018-04-28T21:04:01Z

src/modules/Checkbox/Checkbox.d.ts

+   * @param {object} data - All props and current checked/indeterminate state.
+   */
+  onMouseUp?: (event: React.MouseEvent<HTMLInputElement>, data: CheckboxProps) => void;
+


Relevant change

layershifter · 2018-05-13T12:27:45Z

I've checked this branch, Checkbox doesn't work at all, at example's log I have:

15:25:52: MouseUp       { checked: false } // click again
15:25:52: MouseDown     { checked: false }
15:25:51: MouseUp       { checked: false } // first click
15:25:51: MouseDown     { checked: false }

…React into fix/checkbox-spacebar Signed-off-by: Oleksandr Fediashov <[email protected]> # Conflicts: # docs/app/Examples/modules/Checkbox/States/CheckboxExampleRemoteControl.js # docs/app/Examples/modules/Checkbox/States/index.js # docs/app/Examples/modules/Checkbox/Usage/CheckboxExampleRemoteControl.js # docs/src/examples/modules/Checkbox/States/CheckboxExampleRemoteControl.js # src/modules/Checkbox/Checkbox.d.ts # src/modules/Checkbox/Checkbox.js

Signed-off-by: Oleksandr Fediashov <[email protected]>

…React into fix/checkbox-spacebar

Signed-off-by: Oleksandr Fediashov <[email protected]>

….com/Semantic-Org/Semantic-UI-React into fix/checkbox-spacebar

Signed-off-by: Oleksandr Fediashov <[email protected]>

layershifter · 2018-09-03T14:04:21Z

@levithomason I've fixed tests and replaced some of them with new, also updated example.

There is a single failing conformance test. I think that we can disable it all, what you think?

codecov-io · 2018-12-09T23:04:42Z

Codecov Report

Merging #2748 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2748      +/-   ##
==========================================
+ Coverage   99.89%   99.89%   +<.01%     
==========================================
  Files         170      170              
  Lines        2801     2806       +5     
==========================================
+ Hits         2798     2803       +5     
  Misses          3        3

Impacted Files	Coverage Δ
src/modules/Checkbox/Checkbox.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 132e73f...2121dbd. Read the comment docs.

levithomason · 2018-12-09T23:10:12Z

I've fixed the last broken test. The Checkbox now has full parity with the HTML checkbox. See the new example for confirmation: http://localhost:3000/modules/checkbox/#usage-dom-comparison

The onChange, onClick, onMouseDown, and onMouseUp events are fired in the same order and with the same arguments as a vanilla HTML checkbox.

fix(Checkbox): match DOM checkbox behavior

9c4dec8

levithomason commented Apr 28, 2018

View reviewed changes

levithomason added 2 commits April 28, 2018 14:00

chore(circleci): fix branch ignore config

307172d

fix(Checkbox): add onMouseUp typings

6f1e59e

levithomason mentioned this pull request Apr 28, 2018

Checkbox: doesn't toggle its state, when using only the keyboard #2587

Closed

levithomason commented Apr 28, 2018

View reviewed changes

layershifter added the needs author feedback label May 13, 2018

layershifter mentioned this pull request Jun 21, 2018

Checkbox: Return key is not processed #2931

Closed

Oleksandr Fediashov added 10 commits July 13, 2018 14:36

fix paths

768cf93

Signed-off-by: Oleksandr Fediashov <[email protected]>

Merge branch 'master' of https://github.com/Semantic-Org/Semantic-UI-…

4f669de

…React into fix/checkbox-spacebar

style(Checkbox): fix typings

ec63ed3

Signed-off-by: Oleksandr Fediashov <[email protected]>

style(Checkbox): clean up example

2643da7

Signed-off-by: Oleksandr Fediashov <[email protected]>

Merge branches 'fix/checkbox-spacebar' and 'master' of https://github…

58aa2dd

….com/Semantic-Org/Semantic-UI-React into fix/checkbox-spacebar

docs(Checkbox): add examples with ids

5d3ae00

Signed-off-by: Oleksandr Fediashov <[email protected]>

fix(Checkbox): use onKeyDown

fb62574

Signed-off-by: Oleksandr Fediashov <[email protected]>

fix(Checkbox): back to change, add condition

4bd4543

Signed-off-by: Oleksandr Fediashov <[email protected]>

fix(Checkbox): fix broken tests and add new

dec4685

Signed-off-by: Oleksandr Fediashov <[email protected]>

layershifter added ready for review and removed needs author feedback labels Sep 3, 2018

levithomason added 3 commits December 9, 2018 14:52

Merge branch 'master' into fix/checkbox-spacebar

5d74303

fix failing onClick test

14319e1

enable all tests

b0480d3

levithomason added ready for merge and removed ready for review labels Dec 9, 2018

fix duplicate onClick call

2121dbd

levithomason merged commit d2f74a6 into master Dec 9, 2018

levithomason deleted the fix/checkbox-spacebar branch December 9, 2018 23:12

layershifter mentioned this pull request Dec 23, 2018

Checkbox: double call of onClick when event fired by mouse #3348

Closed

Fabianopb mentioned this pull request Dec 24, 2018

fix(Checkbox): prevent onClick from being called twice #3351

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(Checkbox): match DOM checkbox behavior #2748

fix(Checkbox): match DOM checkbox behavior #2748

levithomason commented Apr 28, 2018 •

edited

Loading

levithomason Apr 28, 2018 •

edited

Loading

levithomason Apr 28, 2018

levithomason Apr 28, 2018 •

edited

Loading

levithomason Apr 28, 2018

layershifter commented May 13, 2018

layershifter commented Sep 3, 2018

codecov-io commented Dec 9, 2018 •

edited

Loading

levithomason commented Dec 9, 2018

fix(Checkbox): match DOM checkbox behavior #2748

fix(Checkbox): match DOM checkbox behavior #2748

Conversation

levithomason commented Apr 28, 2018 • edited Loading

Changes

TODO

levithomason Apr 28, 2018 • edited Loading

Choose a reason for hiding this comment

levithomason Apr 28, 2018

Choose a reason for hiding this comment

levithomason Apr 28, 2018 • edited Loading

Choose a reason for hiding this comment

levithomason Apr 28, 2018

Choose a reason for hiding this comment

layershifter commented May 13, 2018

layershifter commented Sep 3, 2018

codecov-io commented Dec 9, 2018 • edited Loading

Codecov Report

levithomason commented Dec 9, 2018

levithomason commented Apr 28, 2018 •

edited

Loading

levithomason Apr 28, 2018 •

edited

Loading

levithomason Apr 28, 2018 •

edited

Loading

codecov-io commented Dec 9, 2018 •

edited

Loading