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

Add "% Selected" column to transfer list #22131

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

userwiths
Copy link

@userwiths userwiths commented Jan 9, 2025

Closes #22087.

Description

A new column that displays the % of selected data from a torrent.
Its meant to make it easier to distinguish and manage partial torrents as described in the issue.

UI images

Two partial picture of the UI follow, first is from Desktop app (Linux) second is from WebUI (Linux, Mozilla)
image
image

Example use case

You download a few episodes of series or files and want to determine by the first files if its worth downloading the whole torrent. In the presence of multiple torrents it might become bothersome to seek out those partial ones.
With this column you could go in Completed and order by % Selected ascending, which would put the partial torrents on top. From there on its up to the user.

@glassez glassez changed the title feat: Add "% Selected" column. Add "% Selected" column to transfer list Jan 9, 2025
@glassez glassez added WebUI WebUI-related issues/changes GUI GUI-related issues/changes labels Jan 9, 2025
@glassez glassez requested a review from a team January 9, 2025 10:10
@@ -194,6 +194,7 @@ QVariant TransferListModel::headerData(const int section, const Qt::Orientation
case TR_INFOHASH_V2: return tr("Info Hash v2", "i.e: torrent info hash v2");
case TR_REANNOUNCE: return tr("Reannounce In", "Indicates the time until next trackers reannounce");
case TR_PRIVATE: return tr("Private", "Flags private torrents");
case TR_PERCENT_SELECTED: return tr("% Selected", "Percentage of torrent selected");
Copy link
Contributor

Choose a reason for hiding this comment

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

% Selected

Don't know about others but for me, what comes to mind is percentage of files selected not percentage of size.

Copy link
Author

Choose a reason for hiding this comment

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

I can see where the confusion comes from, but I'm not sure how exactly to call it in order to make it clearer.
Changed it to % Selected Data, but I accept other suggestions that make it clearer.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would return “% Selected” or even leave just “%” to minimize the column width as much as possible since there is a tooltip.

Copy link
Author

Choose a reason for hiding this comment

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

As advised, I've changed the text to the shortest version %.

Copy link
Contributor

Choose a reason for hiding this comment

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

Use the tooltip for this column.

Copy link
Contributor

Choose a reason for hiding this comment

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

A plain "%" as column header is completely unacceptable IMO.

Copy link
Contributor

@stalkerok stalkerok Jan 13, 2025

Choose a reason for hiding this comment

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

In any other case, the column content will always be smaller than the column header, can minimize the column header as much as possible and use a tooltip.

Copy link
Contributor

@thalieht thalieht Jan 13, 2025

Choose a reason for hiding this comment

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

can minimize the column header as much as possible and use a tooltip.

That will be the case regardless of the header's string length.
Take for example the # (queue column) there are many issues complaining how the torrent's "index" or something like that column is gone or whatever. Who knows how many people actually don't know that it's the queue position.

src/gui/transferlistmodel.cpp Show resolved Hide resolved
@@ -208,6 +208,7 @@ int TransferListSortModel::compare(const QModelIndex &left, const QModelIndex &r
case TransferListModel::TR_RATIO:
case TransferListModel::TR_RATIO_LIMIT:
case TransferListModel::TR_POPULARITY:
case TransferListModel::TR_PERCENT_SELECTED:
Copy link
Member

Choose a reason for hiding this comment

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

You should put it at the toLongLong() group instead.

Copy link
Member

Choose a reason for hiding this comment

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

Or perhaps you should calculate it in floating point instead of integer.

Copy link
Author

Choose a reason for hiding this comment

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

Changed it so that we .toFloat() for this column before we compare.

src/webui/api/serialize/serialize_torrent.cpp Outdated Show resolved Hide resolved
@@ -166,6 +172,7 @@ QVariantMap serialize(const BitTorrent::Torrent &torrent)
{KEY_TORRENT_COMMENT, torrent.comment()},
{KEY_TORRENT_PRIVATE, (torrent.hasMetadata() ? torrent.isPrivate() : QVariant())},
{KEY_TORRENT_TOTAL_SIZE, torrent.totalSize()},
{KEY_TORRENT_HAS_METADATA, torrent.hasMetadata()}
{KEY_TORRENT_HAS_METADATA, torrent.hasMetadata()},
{KEY_TORRENT_PERCENT_SELECTED, getSelectedPercentage()}
Copy link
Member

Choose a reason for hiding this comment

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

Furthermore, it seems you shouldn't add this field. The percentage can already be calculated from existing data. Just do the calculation with existing data in WebUI.

Copy link
Author

Choose a reason for hiding this comment

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

I cannot find an example of a similar column, and my attempts to add a calculated column result in an empty column and non-executing updateTd.
The closest to a calculated column I see is time_active but it is also present in serialize_torrent.cpp & serialize_torrent.h.
If you know a column that does "the calculation with existing data in WebUI" just tell me which and I will try to implement it likewise.

Copy link
Contributor

Choose a reason for hiding this comment

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

The percentage can already be calculated from existing data.

I'm not defending the extra field but the same can be said about ETA, popularity and amount_left...

@stalkerok
Copy link
Contributor

stalkerok commented Jan 17, 2025

Tested.
You should also change the column title for the WebUI.
And add a tooltip (popularity column, example) if this short title will be approved.

@userwiths userwiths requested a review from stalkerok January 18, 2025 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI GUI-related issues/changes WebUI WebUI-related issues/changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "% Selected" column (= Size / Total Size)
5 participants