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

Remove redundant judgments #212

Merged
merged 1 commit into from May 29, 2020
Merged

Remove redundant judgments #212

merged 1 commit into from May 29, 2020

Conversation

baooab
Copy link
Contributor

@baooab baooab commented May 28, 2020

I deleted the code event.target! == el. The reason is as follows:

  • For path.indexOf(el)

If el is not in event.path,indicates that the click occurred outside el

  • For !el.contains(event.target)

If event.target is not a descendant element of el or el itself,indicates that the click occurred outside el

Therefore, the additional judgment of event.target! == el is not needed.

(Forgive my poor english 😂)

I deleted the code `event.target! == el`. The reason is as follows:

- For `path.indexOf(el)`

If `el` is not in `event.path`,indicates that the click occurred outside `el`

- For `!el.contains(event.target)`

If `event.target` is not a descendant element of `el` or `el` itself,indicates that the click occurred outside `el`

Therefore, the additional judgment of `event.target! == el` is not needed.

(Forgive my English is not good enough 😂)
@coveralls
Copy link

Coverage Status

Coverage increased (+2.7%) to 66.667% when pulling d54d7c7 on baooab:patch-1 into c9cb022 on ndelvalle:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+2.7%) to 66.667% when pulling d54d7c7 on baooab:patch-1 into c9cb022 on ndelvalle:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.7%) to 66.667% when pulling d54d7c7 on baooab:patch-1 into c9cb022 on ndelvalle:master.

Copy link
Owner

@ndelvalle ndelvalle left a comment

Choose a reason for hiding this comment

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

LGTM

@ndelvalle
Copy link
Owner

Thanks for the contribution @baooab, for some external reasons the CI is failing, I'll merge and fix this later.

@ndelvalle ndelvalle merged commit a7c322b into ndelvalle:master May 29, 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

3 participants