WebUI: Allow to move state icon to name column in torrents table #22118
Conversation
|
This is not a good solution. Some users on the contrary request separate columns for the GUI #18193 I would suggest making a full column for WebUI called “Status Icon” or just “Icon” instead of merging. |
I see, thanks for letting me know. It doesn't hurt to keep the old state icon column so I'll modify the PR and bring it back. |
skomerko
force-pushed
the
webui-state-icon-column
branch
from
ad6fafb to
08272b6
Compare
skomerko
changed the title
|
PR has been updated. Notable changes:
edit: PR title probably should be changed(?), not sure to what though. |
|
How about the name “Icon”? #22131 (comment)
|
| @@ -612,31 +628,21 @@ window.qBittorrent.DynamicTable ??= (() => { | |||
| return -1; | |||
| }, | |||
|
|
|||
| updateColumn: function(columnName) { | |||
| updateColumn: function(columnName, updateCellData = false) { | |||
There was a problem hiding this comment.
@Chocobo1, changes made around this method are not strictly necessery - name column could be updated from within onVisibilityChange callback but I thought it's better to improve updateColumn a bit instead (though things are definitely not perfect but I didn't want to change too much).
| this.columns["state_icon"].updateTd = function(td, row) { | ||
| let state = this.getRowValue(row); | ||
| let img_path; | ||
| const getStateIconClasses = (state) => { |
There was a problem hiding this comment.
Shouldn't it be singular?
| const getStateIconClasses = (state) => { | |
| const getStateIconClass = (state) => { |
| @@ -789,6 +795,14 @@ window.qBittorrent.DynamicTable ??= (() => { | |||
| } | |||
| }, | |||
|
|
|||
| getTrs: function() { | |||
| return [...this.tableBody.rows]; | |||
There was a problem hiding this comment.
How about this?
| return [...this.tableBody.rows]; | |
| return this.tableBody.rows; |
It should be enough if you only need iteration.
There was a problem hiding this comment.
Both rows & cells properties return a live HTMLCollection which is automatically updated when the underlying document is changed, so I convert them to a static one to avoid it. IIRC getElements called with a simple tag does the same thing - it uses getElementsByTagName internally and then creates Elements collection (live -> static).
Maybe it would be better to return this.tableBody.querySelectorAll("tr") instead? I can't tell if there is a difference in performance between these two approaches.
| }, | ||
|
|
||
| getRowCells: (tr) => { | ||
| return [...tr.cells]; |
There was a problem hiding this comment.
Here too:
| return [...tr.cells]; | |
| return tr.cells; |
See #22118 (comment) for updated information.