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(Keybinds): Fix keybinds order when recording #1918

Merged
merged 3 commits into from
Mar 12, 2024
Merged

Conversation

lgmarchi
Copy link
Contributor

@lgmarchi lgmarchi commented Mar 11, 2024

What this PR does 📖

1. Fix keybinds order when recording

Order of keybinds will be always like image above. Being "E" any other key is not modifier.

image

Which issue(s) this PR fixes 🔨

Special notes for reviewers 🗒️

Additional comments 🎤

@lgmarchi lgmarchi self-assigned this Mar 11, 2024
@github-actions github-actions bot added Don't merge yet DO NOT MERGE Missing dev review Still needs to be reviewed by a dev Failed Automated Test This PR is failing Luis's Appium test and needs revised labels Mar 11, 2024
Copy link
Contributor

github-actions bot commented Mar 11, 2024

UI Automated Test Results Summary for MacOS/Windows

409 tests   376 ✅  2h 42m 40s ⏱️
 31 suites   29 💤
  3 files      2 ❌  2 🔥

For more details on these failures and errors, see this check.

Results for commit a071d8d.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Mar 11, 2024

UI Automated Tests execution is complete! You can find the test results report here

@luisecm
Copy link
Contributor

luisecm commented Mar 11, 2024

@lgmarchi The automated test currently failing will need to be updated once that this PR is merged, since its expecting that when sending Alt + Ctrl + Shift + P, it will record Alt + Ctrl + Shift + P. Given the change from this PR, the new expected result should be Ctrl + Shift + Alt + P, regardless of the order of push, right?

@lgmarchi
Copy link
Contributor Author

@lgmarchi The automated test currently failing will need to be updated once that this PR is merged, since its expecting that when sending Alt + Ctrl + Shift + P, it will record Alt + Ctrl + Shift + P. Given the change from this PR, the new expected result should be Ctrl + Shift + Alt + P, regardless of the order of push, right?

Perfect @luisecm, we will keep this pattern, I saw other apps, and we will follow always same order, independent of order user typed

@stavares843 stavares843 added the QA Tested QA has tested and approved label Mar 12, 2024
@dariusc93 dariusc93 added Ready to Merge This PR is ready to merge and removed Missing dev review Still needs to be reviewed by a dev Don't merge yet DO NOT MERGE labels Mar 12, 2024
@stavares843 stavares843 merged commit 9145c0a into dev Mar 12, 2024
5 of 7 checks passed
@stavares843 stavares843 deleted the fix-keybinds-order branch March 12, 2024 17:20
@github-actions github-actions bot added Failed Automated Test This PR is failing Luis's Appium test and needs revised and removed QA Tested QA has tested and approved Ready to Merge This PR is ready to merge Failed Automated Test This PR is failing Luis's Appium test and needs revised labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Failed Automated Test This PR is failing Luis's Appium test and needs revised
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

bug(keyBindings): Alt always appears first when mapping keybindings
4 participants