-
-
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
feat(browser): support clipboard api userEvent.copy, cut, paste
#6769
Conversation
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
const code = 'location' in keyDef ? keyDef.key! : keyDef.code! | ||
const special = Key[code as 'Shift'] |
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 changed this as it wasn't working when key === 'Ctrl'
, which is webdriverio's special key to switch modifier keys based on platform. I'm not sure the original code's intent, but it didn't break existing tests.
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.
The difference here is AltrRight
for example - see https://github.com/testing-library/user-event/blob/main/src/keyboard/keyMap.ts. Without this, it will always trigger Alt
, not AltRight
The same is for ControlLeft
/ControlRight
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.
This is broken anyways regardless of what I do here. (see test/browser/fixtures/user-event/keyboard.test.ts
. on main [Shift]
works differently, but [Shift]
shouldn't actually work since there's no such code
).
I don't think solving this is a scope of PR adding copy/cut/paste sugars, so we can follow this up separately #7118.
userEvent.copy, cut, paste
This got lost! Can you resolve the merge conflicts? Feel free to merge! |
Review required 😄 |
const code = 'location' in keyDef ? keyDef.key! : keyDef.code! | ||
const special = Key[code as 'Shift'] |
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.
The difference here is AltrRight
for example - see https://github.com/testing-library/user-event/blob/main/src/keyboard/keyMap.ts. Without this, it will always trigger Alt
, not AltRight
The same is for ControlLeft
/ControlRight
? 'ControlOrMeta' | ||
: provider === 'webdriverio' | ||
? 'Ctrl' | ||
: 'Control' |
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 am confused why this is required here. All sent characters are from https://github.com/testing-library/user-event/blob/main/src/keyboard/keyMap.ts shouldn't we do this check in keyboard
?
We probably need a test for every key 👀
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.
parseKeyDef
doesn't strictly check what's inside {xxx}
and, for example, {ControlOrMeta}
ends up with keyDef: { key: 'ControlOrMeta', code: 'Unknown' }
. Then we only send keyDef.key
to the provider, so it's working.
We should probably do something with this key translation layer, but what do you suggest for this specific ControlOrMeta
etc... specifically? Is this about consistency between providers (like supporting ControlOrMeta
both for playwright and webdriverio)?
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.
Is this about consistency between providers (like supporting ControlOrMeta both for playwright and webdriverio)?
I think it's about the compatibility with testing-library. If there are keys that are unique to providers, they can stay that way, but common keys should do the same thing and should be entered the same way like in the key map *so, ControlOrMeta
is Control
in every provider or something) - we can extend the keyMap, by the way
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.
Okay, that sounds like a good spec. I'll check later, but providers might only support only {key}
concept (like {Control}
), but not physical [code]
(like [ControlLeft]
). Is alignment only for {key}
or you're thinking about to do [code]
too?
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 fine to fallback to Control
when ControlLeft
is used. There is a location
property on the even though, so it might be useful for some: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/location
But if there is no way to do that in provider, then we can't do anything about it yet
0b5c4a9
to
6ff4a4f
Compare
This reverts commit 6ff4a4f.
Description
Manually tested on my Linux PC:
todo
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yaml
unless you introduce a new test example.Tests
pnpm test:ci
.Documentation
pnpm run docs
command.Changesets
feat:
,fix:
,perf:
,docs:
, orchore:
.