Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions docs/guide/browser/interactivity-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,33 @@ test('clicks on an element', async () => {
await userEvent.click(logo)
// or you can access it directly on the locator
await logo.click()

// With WebdriverIO, this uses either ElementClick (with no arguments) or
// actions (with arguments). Use an empty object to force the use of actions.
await logo.click({})
})
```

### Clicking with a modifier

With either WebdriverIO or Playwright:

```ts
await userEvent.keyboard('{Shift>}')
// By using an empty object as the option, this opts in to using a chain of actions
// instead of an ElementClick in webdriver.
// Firefox has a bug that makes this necessary.
// Follow https://bugzilla.mozilla.org/show_bug.cgi?id=1456642 to know when this
// will be fixed.
await userEvent.click(element, {})
await userEvent.keyboard('{/Shift}')
```

With Playwright:
```ts
await userEvent.click(element, { modifiers: ['Shift'] })
```

References:

- [Playwright `locator.click` API](https://playwright.dev/docs/api/class-locator#locator-click)
Expand Down
8 changes: 4 additions & 4 deletions packages/browser-webdriverio/src/commands/click.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ import type { UserEventCommand } from './utils'
export const click: UserEventCommand<UserEvent['click']> = async (
context,
selector,
options = {},
options,
Copy link
Member

Choose a reason for hiding this comment

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

Did you check what is passed here when = {} is removed? I think we always normalize it in the browser/*/locators/index.ts

Maybe a better test could catch that 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah good question, I'll double check at least by adding logs :)

I'm not sure how to do a good test, as they should essentially do the same thing in the end ... except with more work (possibly taking longer especially).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

or maybe I could try to mock the call to see what is really called...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah you're right of course:

public click(options: UserEventClickOptions = {}): Promise<void> {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Playwright defaults to {} too, and the preview one doesn't take any option, so I guess we can remove the default everywhere.

https://github.com/microsoft/playwright/blob/e7bff526433b6dcb02801763ab5b1c6407902d47/packages/playwright-core/src/client/locator.ts#L108

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://bugzilla.mozilla.org/show_bug.cgi?id=1456642 turns out there's a bug open in Firefox, it looks like it's actively worked on these days.

Given there's an easy workaround (passing {} as a parameter to click), I'll leave it up to you to decide if you want to merge this.

Copy link
Member

Choose a reason for hiding this comment

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

I think if we can add a documentation notice to the click event, it's fine to merge. The impact should not be big

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed some documentation, tell me if that looks like what you expected.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, looks good to me, just make sure eslint doesn't fail

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh no, should be fixed now :-)

) => {
const browser = context.browser
await browser.$(selector).click(options as any)
await browser.$(selector).click(options)
}

export const dblClick: UserEventCommand<UserEvent['dblClick']> = async (
context,
selector,
_options = {},
_options,
) => {
const browser = context.browser
await browser.$(selector).doubleClick()
Expand All @@ -22,7 +22,7 @@ export const dblClick: UserEventCommand<UserEvent['dblClick']> = async (
export const tripleClick: UserEventCommand<UserEvent['tripleClick']> = async (
context,
selector,
_options = {},
_options,
) => {
const browser = context.browser
await browser
Expand Down
6 changes: 3 additions & 3 deletions packages/browser/src/client/tester/locators/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,15 @@ export abstract class Locator {
})
}

public click(options: UserEventClickOptions = {}): Promise<void> {
public click(options?: UserEventClickOptions): Promise<void> {
return this.triggerCommand<void>('__vitest_click', this.selector, options)
}

public dblClick(options: UserEventClickOptions = {}): Promise<void> {
public dblClick(options?: UserEventClickOptions): Promise<void> {
return this.triggerCommand<void>('__vitest_dblClick', this.selector, options)
}

public tripleClick(options: UserEventClickOptions = {}): Promise<void> {
public tripleClick(options?: UserEventClickOptions): Promise<void> {
return this.triggerCommand<void>('__vitest_tripleClick', this.selector, options)
}

Expand Down
3 changes: 2 additions & 1 deletion test/browser/fixtures/user-event/keyboard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ test('click with modifier', async () => {
});

await userEvent.keyboard('{Shift>}')
await userEvent.click(el)
// By using an empty object as the option, this opts in to using a chain of actions instead of an elementClick in webdriver.
await userEvent.click(el, {})
await userEvent.keyboard('{/Shift}')
await expect.poll(() => el.textContent).toContain("[ok]")
})
Expand Down
11 changes: 11 additions & 0 deletions test/browser/test/userEvent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,23 @@ describe('userEvent.click', () => {
document.body.appendChild(button)
const onClick = vi.fn()
const dblClick = vi.fn()
// Make sure a contextmenu doesn't actually appear, as it may make some
// tests fail later.
const onContextmenu = vi.fn(e => e.preventDefault())
button.addEventListener('click', onClick)
button.addEventListener('dblclick', onClick)
button.addEventListener('contextmenu', onContextmenu)

await userEvent.click(button)

expect(onClick).toHaveBeenCalled()
expect(dblClick).not.toHaveBeenCalled()
expect(onContextmenu).not.toHaveBeenCalled()

onClick.mockClear()
await userEvent.click(button, { button: 'right' })
expect(onClick).not.toHaveBeenCalled()
expect(onContextmenu).toHaveBeenCalled()
})

test('correctly doesn\'t click on a disabled button', async () => {
Expand Down
Loading