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

fix: use correct control modifier #1294

Closed
wants to merge 1 commit into from
Closed

fix: use correct control modifier #1294

wants to merge 1 commit into from

Conversation

Rafatcb
Copy link

@Rafatcb Rafatcb commented Aug 25, 2023

I was creating a test and {Ctrl} wasn't working. I searched through the code and found that the correct modifier is{Control}, as you can see in user-event/tests/keyboard/parseKeyDef.ts and user-events/tests/utility/type.ts.

@netlify
Copy link

netlify bot commented Aug 25, 2023

Deploy Preview for testing-library ready!

Name Link
🔨 Latest commit bfda769
🔍 Latest deploy log https://app.netlify.com/sites/testing-library/deploys/64e8affbb936570008ae01c9
😎 Deploy Preview https://deploy-preview-1294--testing-library.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@MatanBobi
Copy link
Member

Hi @Rafatcb! Thanks for taking the time to open this one.
Just verifying, you're editing the user-event v13 page, are you using user-event v13?
The code references you've posted here are true for v14, I can't find an answer for v13 - @ph-fritsche is this pr correct? :)

@Rafatcb
Copy link
Author

Rafatcb commented Aug 25, 2023

Hi @MatanBobi.

Sorry, I didn't realize it was v13. I'm using the latest version (14.4.3). I didn't find a documentation for the special characters for v14. I found only the documentation that I suggested editing.

Although the code is for the current version (v14), I checked Git Blame and the original commit is from PR #581, which went into the v13 release.

@MatanBobi
Copy link
Member

Nice backtracking :)
Let's just wait for @ph-fritsche to approve that this indeed is correct for v13.

@ph-fritsche
Copy link
Member

The keyboardMap was introduced in v13.0.0 per testing-library/user-event#581 – this includes the support for {Control}.
The support for {Ctrl} was removed in v14.0.0 per testing-library/user-event#783.

@MatanBobi
Copy link
Member

Thanks @ph-fritsche :)
If that's the case, do you think that the v13 page should be with {Ctrl} as it is today or with {Control}?

@ph-fritsche
Copy link
Member

I think we should keep it as it is. The table is for the (legacy) modifiers which had separate implementations in earlier versions.

The following sentence is only true for {ctrl} but not for {control}:

> **A note about modifiers:** Modifier keys (`{shift}`, `{ctrl}`, `{alt}`,
> `{meta}`) will activate their corresponding event modifiers for the duration
> of type command or until they are closed (via `{/shift}`, `{/ctrl}`, etc.). If

@MatanBobi
Copy link
Member

If that's the case, I think we can close this one. Thanks @Rafatcb for the effort.
Having said that, following this PR it looks like users are looking for a similar table for the current version of user-event :)

@MatanBobi MatanBobi closed this Aug 27, 2023
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.

3 participants