webtorrent-desktop icon indicating copy to clipboard operation
webtorrent-desktop copied to clipboard

feat: remove torrent immediately, but allow Undo

Open dsernst opened this issue 4 years ago • 10 comments

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update [ ] Bug fix [x] New feature [ ] Other, please explain:

What changes did you make? (Give an overview)

This is a continuation of the work started in PR https://github.com/webtorrent/webtorrent-desktop/pull/1087 to replace the Remove a Torrent UI modal with a Snackbar style alert + option to Undo.

Kapture 2020-02-05 at 17 26 10

The 3 other methods of removing a torrent:

  • Right click in list > Remove Data File
  • Transfers menu > Remove All From List
  • Transfers menu > Remove All Data Files

all continue to use the same modal as before (per discussion in https://github.com/webtorrent/webtorrent-desktop/issues/970).

Which issue (if any) does this pull request address?

https://github.com/webtorrent/webtorrent-desktop/issues/970, https://github.com/webtorrent/webtorrent-desktop/pull/1087 (abandoned PR)

👍 This is tested locally and works on my machine running OS X 10.14.6.

dsernst avatar Feb 06 '20 02:02 dsernst

From https://github.com/webtorrent/webtorrent-desktop/issues/970#issuecomment-249908009:

image

This PR doesn't add support for a Ctrl-Z style Undo hotkey.

I wanted to get something working out the door first, & don't think the hotkey ought to be a blocker.

dsernst avatar Feb 06 '20 02:02 dsernst

One important thing to note:

The Undo feature currently only works for magnet links, which doesn't include the Featured torrents on the WebTorrent Desktop homepage, so I don't know how to demo that properly.

✅ I confirmed that the Undo feature does work on a torrent with good magnet links.

  • [ ] If someone can point me to a good public demo torrent w/ magnet support, I can make a quick gif demo of that to show it resuming etc.

Even better would be to fix this up so the Undo works for torrents added via a .torrent file.

I can work on that, but would appreciate a little advice on how best to approach.

Currently, the locally stored ${appConfig}/Torrents/ files are still being deleted immediately.

  • So one solution is to simply delay deleting the .torrent files until after the Snackbar autohides itself after 5 seconds. That ought to work, but if the Application quits before the 5 second timer, the file may never get cleaned up, which doesn't seem great.

  • A different solution is to remember how the torrent was added in first place, such as via pasting in a link like https://archive.org/download/art_of_war_librivox/art_of_war_librivox_archive.torrent. Then the Undo call could go try to add that way again. But as far as I could tell, these links aren't being stored within torrentSummary right now, just the path to the new local .torrent file.

dsernst avatar Feb 06 '20 02:02 dsernst

I tested this locally and the Undo option also worked for the Featured torrents in WebTorrent.

System: Ubuntu 18.04

subins2000 avatar May 12 '20 17:05 subins2000

Fixed merge conflict

dsernst avatar Jul 18 '20 10:07 dsernst

Best path forward, per discussion in today's Webtorrent Maintainers call:

So one solution is to simply delay deleting the .torrent files until after the Snackbar autohides itself after 5 seconds. That ought to work, but if the Application quits before the 5 second timer, the file may never get cleaned up, which doesn't seem great.

We can add a flag like pendingDelete to the torrent. The UI will hide these items, so from the user's perspective it's been deleted. Then have a function like removePendingDeletes that goes through and actually deletes it from the state. That function can be called after the Undo's timer ends, as well as on app shutdown and app start (in case the app had crashed).

dsernst avatar Dec 04 '20 18:12 dsernst

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

github-actions[bot] avatar Jul 22 '21 12:07 github-actions[bot]

Yes it’s still relevant, if we want to simplify the UI (replace two click modal w/ one click undoable).

Nothing’s changed since my last comment.

  • [x] This works great for .torrent files.
  • [ ] Undo doesn’t work for magnet links yet. There is a relatively clear path forward.

I just haven’t taken the time to work on this lately, as I’ve been focused on other projects 😄

dsernst avatar Jul 22 '21 12:07 dsernst

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

github-actions[bot] avatar Sep 22 '21 12:09 github-actions[bot]

@dsernst What's the path forward of with magnets, I'll take it on myself

DiegoRBaquero avatar Oct 19 '21 00:10 DiegoRBaquero

@dsernst What's the path forward of with magnets, I'll take it on myself

Awesome to hear. 🎉

Last I spoke about this w/ @feross, he liked this approach:

Best path forward, per discussion in today's Webtorrent Maintainers call:

So one solution is to simply delay deleting the .torrent files until after the Snackbar autohides itself after 5 seconds. That ought to work, but if the Application quits before the 5 second timer, the file may never get cleaned up, which doesn't seem great.

We can add a flag like pendingDelete to the torrent. The UI will hide these items, so from the user's perspective it's been deleted. Then have a function like removePendingDeletes that goes through and actually deletes it from the state. That function can be called after the Undo's timer ends, as well as on app shutdown and app start (in case the app had crashed).

dsernst avatar Oct 19 '21 01:10 dsernst