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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(190): Add blur events in correct order #218

Closed
wants to merge 1 commit into from

Conversation

jsmapr1
Copy link

@jsmapr1 jsmapr1 commented Feb 13, 2020

Blur occurs after mouse down, but before click.
I can't find any official documentation, but here's a nice code pen (not using library):
https://codepen.io/mudassir0909/pen/eIHqB

This is important because an onBlur event in React happens before onClick and if it rerenders the element, the onClick event never happens.

This covered a case from a previous PR #208, so I remove the related code.

Let me know if you need anything else. 馃帀

Blur occurs after mouse down, but before click. I can't find any
official documentation, but here's a nice code pen:
https://codepen.io/mudassir0909/pen/eIHqB
fireEvent.mouseDown(element);
element.focus();
const continueDefaultHandling = fireEvent.mouseDown(element);
if (continueDefaultHandling) {
Copy link
Author

Choose a reason for hiding this comment

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

Added to be consistent with single click

@codecov
Copy link

codecov bot commented Feb 13, 2020

Codecov Report

Merging #218 into master will increase coverage by 1.36%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #218      +/-   ##
=========================================
+ Coverage   97.24%   98.6%   +1.36%     
=========================================
  Files           1       1              
  Lines         145     143       -2     
  Branches       35      41       +6     
=========================================
  Hits          141     141              
+ Misses          4       2       -2
Impacted Files Coverage 螖
src/index.js 98.6% <100%> (+1.36%) 猬嗭笍

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 efefe15...86a4d60. Read the comment docs.

@@ -101,15 +105,6 @@ function fireChangeEvent(event) {
event.target.removeEventListener("blur", fireChangeEvent);
}

function blurFocusedElement(element, focusedElement, wasAnotherElementFocused) {
Copy link
Author

Choose a reason for hiding this comment

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

Now that we focus before click, we don't need to blur elements at the end of the actions.

@Gpx Gpx mentioned this pull request Feb 17, 2020
@Gpx
Copy link
Member

Gpx commented Feb 17, 2020

Merged in #218, thanks!

@Gpx Gpx closed this Feb 17, 2020
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.

None yet

2 participants