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

Include event.detail in click and dblclick events #1156

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andrewtimberlake
Copy link

This change updates UIEvent.detail (https://developer.mozilla.org/en-US/docs/Web/API/UIEvent/detail) on click and dblclick events.

@kategengler
Copy link
Member

From the documentation linked:

For click or dblclick events, UIEvent.detail is the current click count.

From some quick experimentation, successive clicks on a button with a click handler yielded an event.detail of 1,2,3,4,5,6...

Knowing that, I would expect that we wouldn't always want UIEvent.detail on a click to be 1, as you could have successive click events in a tests.

For mousedown or mouseup events, UIEvent.detail is 1 plus the current click count.

I haven't tested this, but it sounds like the detail option that is passed along to mousedown and mouseup would need to increment.

@@ -33,6 +33,7 @@ export function __click__(
element: Element | Document | Window,
options: MouseEventInit
): void {
options = { ...options, detail: 1 };
Copy link
Member

Choose a reason for hiding this comment

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

From https://github.com/emberjs/ember-test-helpers/blob/master/API.md#click

I believe this would be a breaking change as users could no longer override detail on the event.

Copy link
Author

Choose a reason for hiding this comment

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

That is a good point.

Copy link
Member

Choose a reason for hiding this comment

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

ya, you'd have to do it the other way around (so the users provided detail option "wins"):

Suggested change
options = { ...options, detail: 1 };
options = { detail: 1, ...options};

@andrewtimberlake
Copy link
Author

For mousedown or mouseup events, UIEvent.detail is 1 plus the current click count.

I haven't tested this, but it sounds like the detail option that is passed along to mousedown and mouseup would need to increment.

The spec here sounds confusing but the increment is happening. Because mouse down and mouse up trigger before click they start at 0 (click) + 1, then 1 click + 1 etc.

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