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

feat: can dispatch events with the type they want. #8305

Merged
merged 13 commits into from Aug 31, 2020

Conversation

sainthkh
Copy link
Contributor

@sainthkh sainthkh commented Aug 18, 2020

User facing changelog

Users can dispatch events with the type they want. And the event is an instance of UIEvent, view property is filled with AUT window.

Additional details

Why was this change necessary?

  • Some users want to use the correct type of Event with cy.trigger(). But it was impossible.
  • view property didn't replicate the behavior of the browsers.

What is affected by this change?

N/A

Any implementation details to explain?

At first, I tried to implement this with an object map like below:

const eventMap = {
  'mousedown': 'MouseEvent',
  'keydown': 'KeyboardEvent',
  etcetra,
}

Because trigger can find the correct automatically. But the problem is that most of the properties of Events are read-only. They cannot be modified even with _.extend.

It's a big problem because you cannot fire MouseEvent like:

let options = {
        clientX: 42,
        clientY: 24,
        pageX: 420,
        pageY: 240,
        foo: 'foo',
      }

MouseEvent constructor doesn't get pageX and pageY properties and they are automatically set to the values of clientX and clientY.

So, I had to give up eventMap and introduce eventType option instead.

How has the user experience changed?

cy.$$('input').then($el => {
  $el.get(0).addEventListener('mousedown', (e) => {
    // e is an instance of AUT window MouseEvent
    // e.view is AUT window.
  })
})

cy.get('input').trigger('mousedown', {
  eventType: 'MouseEvent'
})

PR Tasks

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 18, 2020

Thanks for taking the time to open a PR!

chrisbreiding
chrisbreiding previously approved these changes Aug 25, 2020
@jennifer-shehane
Copy link
Member

I'm not totally in love with calling this option eventType since there is a literal event.type that would be the name of the event.

But, looking through all the docs for this it seems they mostly refer to this as an 'interface', see https://w3c.github.io/uievents/#interface-keyboardevent

Would an 'interface' key even make sense? Probably not. @chrisbreiding any thoughts? Maybe the current naming is harmless.

@chrisbreiding
Copy link
Contributor

I'm not totally in love with calling this option eventType since there is a literal event.type that would be the name of the event.

I agree that's problematic and could be confusing.

I don't think I like interface though. It's more of a keyword used for type definitions.

We could use constructor since that's exactly what this property refers to.

cy.trigger('mousedown', {
  constructor: 'MouseEvent',
})

Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

@sainthkh What do you think of the name constructor for this new option?

@sainthkh
Copy link
Contributor Author

I think it's better. It's less confusing. I'll change it that way.

@sainthkh
Copy link
Contributor Author

sainthkh commented Aug 28, 2020

constructor is always defined in browsers. That's why there are a lot of failures below.

You can check it out by just opening a console window and try the code below:

Screenshot from 2020-08-28 16-13-38

Screenshot from 2020-08-28 16-13-43

I thought of writing code like below to avoid this problem, but it's too unintuitive.

const constructor = typeof options.constructor === 'string' ? options.constructor : 'Event' 

I came to the conclusion that we cannot use the name, constructor.

My proposal: How about naming it eventClassName or eventInterfaceName?

@jennifer-shehane
Copy link
Member

😞 maybe eventConstructor? @chrisbreiding thoughts?

@chrisbreiding
Copy link
Contributor

I was worried that would be an issue. Let's go with eventConstructor.

@sainthkh
Copy link
Contributor Author

Flaky failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants