-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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(browser): cleanup keyboard state #6731
Changes from 11 commits
45b40b3
f31dffa
84bc854
37aa0af
4a75432
423b2ec
bfbba25
616ca65
231877d
983ea44
cf0ce91
5941743
f97ee5c
f9f95c9
b23c8ae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,8 @@ function triggerCommand<T>(command: string, ...args: any[]) { | |
return rpc().triggerCommand<T>(contextId, command, filepath(), args) | ||
} | ||
|
||
export function createUserEvent(__tl_user_event__?: TestingLibraryUserEvent): UserEvent { | ||
export function createUserEvent(__tl_user_event_base__?: TestingLibraryUserEvent): UserEvent { | ||
let __tl_user_event__ = __tl_user_event_base__?.setup({}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand why this is needed; the first argument is supposed to be called with setup() - see the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if this what you're asking, but we need to pass around the original import { userEvent } from "@testing-library/user-event"
const userEvent1 = userEvent.setup();
userEvent1.keyboard("{Alt>}")
// userEvent1 and userEvent2 share the same keyboard state
const userEvent2 = userEvent1.setup();
userEvent2.keyboard("{Tab}") // altKey = true
// need to call original `userEvent.setup()` to have a fresh state
{
const userEvent1 = userEvent.setup();
userEvent1.keyboard("{Alt>}")
const userEvent2 = userEvent.setup();
userEvent2.keyboard("{Tab}") // altKey = false
} For L40, we should probably recurse it like |
||
const keyboard = { | ||
unreleased: [] as string[], | ||
} | ||
|
@@ -38,6 +39,14 @@ export function createUserEvent(__tl_user_event__?: TestingLibraryUserEvent): Us | |
setup(options?: any) { | ||
return createUserEvent(__tl_user_event__?.setup(options)) | ||
}, | ||
async cleanup() { | ||
if (typeof __tl_user_event__ !== 'undefined') { | ||
__tl_user_event__ = __tl_user_event_base__?.setup({}) | ||
return | ||
} | ||
await triggerCommand('__vitest_cleanup', keyboard) | ||
keyboard.unreleased = [] | ||
}, | ||
click(element: Element | Locator, options: UserEventClickOptions = {}) { | ||
return convertToLocator(element).click(processClickOptions(options)) | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
import { expect, onTestFinished, test } from 'vitest' | ||
import { userEvent } from '@vitest/browser/context' | ||
|
||
test('cleanup1', async () => { | ||
let logs: any[] = []; | ||
function handler(e: KeyboardEvent) { | ||
logs.push([e.key, e.altKey]); | ||
}; | ||
document.addEventListener('keydown', handler) | ||
onTestFinished(() => { | ||
document.removeEventListener('keydown', handler); | ||
}) | ||
|
||
await userEvent.keyboard('{Tab}') | ||
await userEvent.keyboard("{Alt>}") | ||
expect(logs).toMatchInlineSnapshot(` | ||
[ | ||
[ | ||
"Tab", | ||
false, | ||
], | ||
[ | ||
"Alt", | ||
true, | ||
], | ||
] | ||
`) | ||
}) | ||
|
||
// test per-test cleanup | ||
test('cleanup1.2', async () => { | ||
let logs: any[] = []; | ||
function handler(e: KeyboardEvent) { | ||
logs.push([e.key, e.altKey]); | ||
}; | ||
document.addEventListener('keydown', handler) | ||
onTestFinished(() => { | ||
document.removeEventListener('keydown', handler); | ||
}) | ||
|
||
await userEvent.keyboard('{Tab}') | ||
await userEvent.keyboard("{Alt>}") | ||
expect(logs).toMatchInlineSnapshot(` | ||
[ | ||
[ | ||
"Tab", | ||
false, | ||
], | ||
[ | ||
"Alt", | ||
true, | ||
], | ||
] | ||
`) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import { expect, onTestFinished, test } from 'vitest' | ||
import { userEvent } from '@vitest/browser/context' | ||
|
||
// test per-test-file cleanup just in case | ||
|
||
test('cleanup2', async () => { | ||
let logs: any[] = []; | ||
function handler(e: KeyboardEvent) { | ||
logs.push([e.key, e.altKey]); | ||
}; | ||
document.addEventListener('keydown', handler) | ||
onTestFinished(() => { | ||
document.removeEventListener('keydown', handler); | ||
}) | ||
|
||
await userEvent.keyboard('{Tab}') | ||
await userEvent.keyboard("{Alt>}") | ||
expect(logs).toMatchInlineSnapshot(` | ||
[ | ||
[ | ||
"Tab", | ||
false, | ||
], | ||
[ | ||
"Alt", | ||
true, | ||
], | ||
] | ||
`) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import { fileURLToPath } from 'node:url' | ||
import { defineConfig } from 'vitest/config' | ||
|
||
const provider = process.env.PROVIDER || 'playwright' | ||
const name = | ||
process.env.BROWSER || (provider === 'playwright' ? 'chromium' : 'chrome') | ||
|
||
export default defineConfig({ | ||
cacheDir: fileURLToPath(new URL("./node_modules/.vite", import.meta.url)), | ||
test: { | ||
browser: { | ||
enabled: true, | ||
provider, | ||
name, | ||
}, | ||
}, | ||
}) |
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.
We need documentation for every public API - in jsdoc and vitest.dev