-
Notifications
You must be signed in to change notification settings - Fork 165
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
fix ''event.preventDefault()' call does not change the 'event.defaultPrevented' property value ' (close #1588) #1589
Conversation
…Prevented' property value ' (close DevExpress#1588)
✅ Tests for the commit 4c21336 have passed. See details: |
// NOTE: the dispatchEvent method does not return false in the case when preventDefault method | ||
// was called for events that were created with the KeyboardEvent constructor | ||
if (this.browserWithNewEventsStyle) { | ||
ev.preventDefault = function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to use already implemented mock from https://github.com/DevExpress/testcafe-hammerhead/blob/master/src/client/sandbox/event/index.js#L87
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, we can not use this wrapper by architecture problem:
EventSimulator
is a component of EventSandbox
. And of ElementEditingWatcher
, StorageSandbox
and UploadSandbox
too.
It turns out that we should add link to the created class (EventSandbox
) from its parameter (EventSimulator
).
see https://github.com/DevExpress/testcafe-hammerhead/blob/master/src/client/sandbox/index.js#L57
So, it's better to duplicate code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add comment about it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, let's keep as it is now, but please, open an issue about refactoring this piece of code. It's a pity that we can't use more reliable prototype-level overload here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…Prevented' property value ' (close DevExpress#1588) (DevExpress#1589) * fix ''event.preventDefault()' call does not change the 'event.defaultPrevented' property value ' (close DevExpress#1588) * remove unnecessary new line * remove unused code * remark
@helen-dikareva @AndreyBelym
Fix for DevExpress/testcafe#2070