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

change alt to control for shortcuts #5054

Merged
merged 2 commits into from Feb 26, 2023
Merged

Conversation

tessus
Copy link
Contributor

@tessus tessus commented Feb 25, 2023

I changed the modifier key from Alt to Control, which makes it possible to use it on macOS.

The Deselect all shortcut has also been changed from Control+A to Control+D.

fixes #4829

@ckerr
Copy link
Member

ckerr commented Feb 26, 2023

@tessus Thanks you for doing this! ❤️

Since it's easier for you to test (since you have a Mac) could you please confirm that all these shortcuts work on both Safari and Chrome?

IIRC there was some potential conflict e.g. with Ctrl+O being the browser's "open" as well but TBH I'm fuzzy on the details and might be misremembering -- so the shortest path is to just test and make sure.

I believe FF behaves similarly to Chrome so testing on Safari & Chrome should be sufficient

@ckerr ckerr added the needs testers Call for someone to see if they can reproduce an issue label Feb 26, 2023
@tessus
Copy link
Contributor Author

tessus commented Feb 26, 2023

Yep, it works on Firefox, Chrome, and Safari.

@tessus
Copy link
Contributor Author

tessus commented Feb 26, 2023

Hmm, I just saw that I cannot commit the generated file. I did that only, because it wsa in the source tree. Shall I remove it from my PR?

@ckerr
Copy link
Member

ckerr commented Feb 26, 2023

Hmm, I just saw that I cannot commit the generated file. I did that only, because it wsa in the source tree. Shall I remove it from my PR?

Yes, please.

I don't think we've ever had anyone submit a malicious patch, but webpacked files are difficult to review. Letting the bots run webpack is safer than doing manual review.

@tessus
Copy link
Contributor Author

tessus commented Feb 26, 2023

Done.

@ckerr ckerr added scope:web type:fix A bug fix and removed needs testers Call for someone to see if they can reproduce an issue labels Feb 26, 2023
@ckerr ckerr added this to the 4.0.2 milestone Feb 26, 2023
@tessus
Copy link
Contributor Author

tessus commented Feb 26, 2023

I'm slightly confused why you committed my changes again. Now they are no longer gpg signed. Anyway, not that important, just curious.

@ckerr ckerr merged commit 0d35580 into transmission:main Feb 26, 2023
@ckerr
Copy link
Member

ckerr commented Feb 26, 2023

@tessus I was just rebasing them on top of the lastest commit in main, that's all

@tessus
Copy link
Contributor Author

tessus commented Feb 26, 2023

I see, thanks for the info. I usually only rebase in a branch when there are conflicts, which is why I didn't understand why a rebase was done.

@ckerr
Copy link
Member

ckerr commented Feb 27, 2023

Notes: Fixed broken keyboard shortcuts on desktop Safari.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

alt-i keyboard shortcut for torrent inspector doesn't work
2 participants