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

UnitControl: Fix an issue where keyboard shortcuts unintentionally shift focus on Windows OS #62988

Merged
merged 4 commits into from
Jul 2, 2024

Conversation

t-hamano
Copy link
Contributor

@t-hamano t-hamano commented Jun 29, 2024

Follow-up #39303
Fixes #62967

What?

This PR fixes an issue where the focus unintentionally moves to the unit when executing the paste shortcut (Ctrl+V) in an input field on Windows OS.

Why?

Only event.metaKey is checked to give focus to the unit, which may work on MacOS, but on Windows OS the Ctrl key is event.ctrlKey.

Testing Instructions

The following are test procedures for Windows OS, but they should continue to work as expected on MacOS.

  • Run the Storybook.
  • Access localhost:50240/?path=/docs/components-experimental-unitcontrol--docs
  • Copy the numbers beforehand.
  • Run Ctrl+V in the input field.
  • The copied numbers should be pasted.
  • Press only the p, v or e key.
  • The focus should move to the unit, and the unit should change according to the key pressed.

Screenshots or screencast

After.mp4

@t-hamano t-hamano added [Type] Bug An existing feature does not function as intended [Package] Components /packages/components OS Issues Issues or PRs that are related to OS specific problems labels Jun 29, 2024
@t-hamano t-hamano self-assigned this Jun 29, 2024
@t-hamano t-hamano requested a review from a team June 29, 2024 05:28
@t-hamano t-hamano marked this pull request as ready for review June 29, 2024 05:28
@t-hamano t-hamano requested a review from ajitbohra as a code owner June 29, 2024 05:28
Copy link

github-actions bot commented Jun 29, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @webexpr-dhenriet.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: webexpr-dhenriet.

Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link

Flaky tests detected in cbe9098.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9721840478
📝 Reported issues:

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @t-hamano 🙌

I'd only update the inline comment above your change before shipping.

Otherwise, ready to 🚀

@@ -172,6 +172,7 @@ function UnforwardedUnitControl(
// matches the first character of a unit.
if (
! event.metaKey &&
! event.ctrlKey &&
Copy link
Member

Choose a reason for hiding this comment

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

Let's also update the comment above (to reference the ctrl key in addition to the meta key).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review! Updated by b90cc5a

@t-hamano t-hamano merged commit a19348b into trunk Jul 2, 2024
61 checks passed
@t-hamano t-hamano deleted the unit-control-windows-shortcut branch July 2, 2024 10:45
@github-actions github-actions bot added this to the Gutenberg 18.8 milestone Jul 2, 2024
carstingaxion pushed a commit to carstingaxion/gutenberg that referenced this pull request Jul 18, 2024
…ift focus on Windows OS (WordPress#62988)

* UnitControl: Fix an issue where keyboard shortcuts unintentionally shift focus on Windows OS

* Update changelog

* Update comment

Unlinked contributors: webexpr-dhenriet.

Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS Issues Issues or PRs that are related to OS specific problems [Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Input] - Impossible to paste a value in all input fields with CTRL +V
2 participants