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.

Sign up for GitHub

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
Open

Add "% Selected" column to transfer list #22131

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3d8d163
Feat: Add column that displays the percent of selected data from a to…
userwiths Jan 9, 2025
25238ee
return previosly deleted empty lines
userwiths Jan 9, 2025
765963c
Add metadata check and address PR comments.
userwiths Jan 10, 2025
0234ec8
Pass number to WebUI instead of text and use float when comparing.
userwiths Jan 10, 2025
3618b0f
Shorten column name
userwiths Jan 13, 2025
c203d18
Add tooltip and change webui column name.
userwiths Jan 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/gui/transferlistmodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 •
edited
Loading

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 •
edited
Loading

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.

default: return {};
}
}
Expand Down Expand Up @@ -443,6 +444,8 @@ QString TransferListModel::displayValue(const BitTorrent::Torrent *torrent, cons
return reannounceString(torrent->nextAnnounce());
case TR_PRIVATE:
return privateString(torrent->isPrivate(), torrent->hasMetadata());
case TR_PERCENT_SELECTED:
return QString::number((torrent->wantedSize() * 100) / torrent->totalSize()) + u'%';
thalieht marked this conversation as resolved.
Show resolved Hide resolved
}

return {};
Expand Down Expand Up @@ -526,6 +529,8 @@ QVariant TransferListModel::internalValue(const BitTorrent::Torrent *torrent, co
return torrent->nextAnnounce();
case TR_PRIVATE:
return (torrent->hasMetadata() ? torrent->isPrivate() : QVariant());
case TR_PERCENT_SELECTED:
return (torrent->wantedSize() * 100) / torrent->totalSize();
}

return {};
Expand Down
1 change: 1 addition & 0 deletions src/gui/transferlistmodel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ class TransferListModel final : public QAbstractListModel
TR_INFOHASH_V2,
TR_REANNOUNCE,
TR_PRIVATE,
TR_PERCENT_SELECTED,

NB_COLUMNS
};
Expand Down
1 change: 1 addition & 0 deletions src/gui/transferlistsortmodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

return customCompare(leftValue.toReal(), rightValue.toReal());

case TransferListModel::TR_STATUS:
Expand Down
1 change: 1 addition & 0 deletions src/gui/transferlistwidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ TransferListWidget::TransferListWidget(IGUIApplication *app, QWidget *parent)
setColumnHidden(TransferListModel::TR_TOTAL_SIZE, true);
setColumnHidden(TransferListModel::TR_REANNOUNCE, true);
setColumnHidden(TransferListModel::TR_PRIVATE, true);
setColumnHidden(TransferListModel::TR_PERCENT_SELECTED, true);
}

//Ensure that at least one column is visible at all times
Expand Down
9 changes: 8 additions & 1 deletion src/webui/api/serialize/serialize_torrent.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ QVariantMap serialize(const BitTorrent::Torrent &torrent)
: (QDateTime::currentSecsSinceEpoch() - timeSinceActivity);
};

const auto getSelectedPercentage = [&torrent]() -> QString
{
auto percent = ((torrent.wantedSize() * 100)/torrent.totalSize());
thalieht marked this conversation as resolved.
Show resolved Hide resolved
return QString::number(percent) + u'%';
};

return {
{KEY_TORRENT_ID, torrent.id().toString()},
{KEY_TORRENT_INFOHASHV1, torrent.infoHash().v1().toString()},
Expand Down Expand Up @@ -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()}
Chocobo1 marked this conversation as resolved.
Show resolved Hide resolved
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...

};
}
1 change: 1 addition & 0 deletions src/webui/api/serialize/serialize_torrent.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,5 +96,6 @@ inline const QString KEY_TORRENT_REANNOUNCE = u"reannounce"_s;
inline const QString KEY_TORRENT_COMMENT = u"comment"_s;
inline const QString KEY_TORRENT_PRIVATE = u"private"_s;
inline const QString KEY_TORRENT_HAS_METADATA = u"has_metadata"_s;
inline const QString KEY_TORRENT_PERCENT_SELECTED = u"percent_selected"_s;

QVariantMap serialize(const BitTorrent::Torrent &torrent);
2 changes: 1 addition & 1 deletion src/webui/webapplication.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
#include "base/utils/version.h"
#include "api/isessionmanager.h"

inline const Utils::Version<3, 2> API_VERSION {2, 11, 3};
inline const Utils::Version<3, 2> API_VERSION {2, 11, 4};

class QTimer;

Expand Down
1 change: 1 addition & 0 deletions src/webui/www/private/scripts/dynamicTable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1025,6 +1025,7 @@ window.qBittorrent.DynamicTable ??= (() => {
this.newColumn("infohash_v2", "", "QBT_TR(Info Hash v2)QBT_TR[CONTEXT=TransferListModel]", 100, false);
this.newColumn("reannounce", "", "QBT_TR(Reannounce In)QBT_TR[CONTEXT=TransferListModel]", 100, false);
this.newColumn("private", "", "QBT_TR(Private)QBT_TR[CONTEXT=TransferListModel]", 100, false);
this.newColumn("percent_selected", "", "QBT_TR(Percent Selected)QBT_TR[CONTEXT=TransferListModel]", 100, false);

this.columns["state_icon"].onclick = "";
this.columns["state_icon"].dataProperties[0] = "state";
Expand Down