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

client: add CMD-A, CTRL-A to select all torrents #253

Merged
merged 1 commit into from Mar 22, 2021

Conversation

sabersalv
Copy link
Contributor

@sabersalv sabersalv commented Mar 22, 2021

Description

After the user selected a torrent, user can use CMD-A or CTRL-A to select all torrents. Click one torrent to deselect all. Very useful for bulk editing large collection of torrents.

For #122 (reply in thread)

Types of changes

  • Breaking change (changes that break backward compatibility of public API or CLI - semver MAJOR)
  • New feature (non-breaking change which adds functionality - semver MINOR)
  • Bug fix (non-breaking change which fixes an issue - semver PATCH)

@codecov
Copy link

codecov bot commented Mar 22, 2021

Codecov Report

Merging #253 (449364c) into master (afaccbd) will not change coverage.
The diff coverage is n/a.

❗ Current head 449364c differs from pull request most recent head 987aa1d. Consider uploading reports for the commit 987aa1d to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #253   +/-   ##
=======================================
  Coverage   78.63%   78.63%           
=======================================
  Files          57       57           
  Lines        9211     9211           
  Branches      909      908    -1     
=======================================
  Hits         7243     7243           
  Misses       1957     1957           
  Partials       11       11           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afaccbd...987aa1d. Read the comment docs.

@jesec
Copy link
Owner

jesec commented Mar 22, 2021

Good idea.

I move the event handler to the list component, and attach it to the entire window. I think that would make it more consistent.

The PR works fine for me so far. Please test and let me know when it is good to go.

@@ -47,6 +49,15 @@ const TorrentList: FC = observer(() => {
return dispose;
}, []);

useEvent('keydown', (e: KeyboardEvent) => {
const {repeat, metaKey, ctrlKey, key} = e;
if (repeat) return;
Copy link
Contributor Author

@sabersalv sabersalv Mar 22, 2021

Choose a reason for hiding this comment

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

 const { target } = e;
 if (['INPUT', 'TEXTAREA'].includes(target.tagName)) return;

As it's moved to window, make an exception here, so users can still select all text on <input> and <textarea>

@sabersalv
Copy link
Contributor Author

sabersalv commented Mar 22, 2021

Tested, works great. Thanks for the updated code. Left one comment.

@jesec jesec merged commit 02aa24e into jesec:master Mar 22, 2021
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.

None yet

2 participants