search

lighterowl / transgui

A fork of Transmission Remote GUI
GNU General Public License v2.0
152 stars 4 forks source link

Columns sorting - stay on the first result #25

Closed btTeddy closed 1 year ago

btTeddy commented 1 year ago

Windows 10, latest transgui as of 14/05/2023

Current behaviour:

Proposed behaviour: Clicking on any column will unselect current torrent and so the first result will be on the top.

Actually quite hard to explain properly. Basically it should behave like this example list: https://hdvinnie.github.io/Private-Trackers-Spreadsheet/

lighterowl commented 1 year ago

Thanks for the report. Yeah, I understand what you mean and it makes a lot of sense. It's actually the default behaviour in all other GUIs (i.e. not just torrent clients), I've just gotten so used to this particular quirk in TransGUI that I don't notice it anymore.

lighterowl commented 1 year ago

Unfortunately, it doesn't look like it's possible to have nothing selected in TVarGrid if there's at least one row in it, or I can't figure out how to do this. The current code for sorting does preserve the current selection by saving its state in RowOptions and then restoring it after the sorting is done. However, LCL code in TCustomGrid.SetRow calls MoveExtend with ForceFullyVisible set to true, which then calls ScrollToCell which, unsurprisingly, scrolls the viewport so the selected row is visible. It doesn't look like this behaviour can be made runtime-dependent without making changes to LCL, so at least for now, setting the row means that the grid will automatically scroll to the selected row.

I disabled behaviour by setting the row to -1 after sorting if an extra property is set. This means that the first row in the table will be automatically selected after sorting, which may not be perfect but is still better than the current behaviour, I think.

The new behaviour is only active for the torrents table, I chose to keep the old behaviour in all other lists as I suspect not everybody will be 100% happy about this change.

You can download a build with this change at https://github.com/xavery/transgui/suites/12912061047/artifacts/697190047 .

btTeddy commented 1 year ago

Thanks for trying to get in to it. Really appreciate.

It works... kind of. New behaviour works for a while, then after clicking couple of time on name label etc sorting completely stops to work. Literally does nothing. Manage to repeat the issue twice.... couldn't when I tried 3rd time. Lets call it success.

Dzieki

btTeddy commented 1 year ago

New info. This just showed up image

lighterowl commented 1 year ago

Yeah, it's flaky as hell on Windows, I could see what you're describing as well. Worked fine when I tested it on Linux but that doesn't really help.

I have no other idea on how to approach this right now, sorry.

lighterowl commented 1 year ago

I gave this another shot, will need some more work but looks promising so far. I didn't want to patch LCL code just for TransGUI but it seems like there's no other way and we're building everything from scratch anyway so why not.

@Beersteddy Can you try out this build? https://github.com/xavery/transgui/suites/13024903673/artifacts/705579187

btTeddy commented 1 year ago

@xavery Testing it right now. I'll report any problems found.

Previous build wasn't that bad actually. Crashed once a while when left in background, but nothing major really. It was actually a good feature reminding me to close it :) Really appreciate that you're trying to get it fixed and that you continue abandoned project. Transgui is really outstanding for TM.

BTW. Do you think is possible to convert transgui to webUI for transmission - just like combustion?

lighterowl commented 1 year ago

I don't do any web development so I'm not really the right person to ask. Lazarus has something called Pas2JS but I have no idea what it really does.

btTeddy commented 1 year ago

No worries.

So far I did have one single crash when left transgui running in the background for a very long while. Overall works well enough for what I need. Hopefully one day someone gonna port it to webui. image

lighterowl commented 1 year ago

Very glad to hear, as mentioned before this needs some more work but I should be merging it this week if nothing comes up.

lighterowl commented 1 year ago

No worries.

So far I did have one single crash when left transgui running in the background for a very long while. Overall works well enough for what I need. Hopefully one day someone gonna port it to webui. image

This seems to be https://github.com/transmission-remote-gui/transgui/issues/1459 , so not necessarily related to my changes. Interestingly, the submitter reports using Lazarus 2.2.6.0, and I don't remember this issue occurring with official transgui builds which were built with older versions.

Anyhow, this is now merged by using my own fork of Lazarus/LCL which allows skipping the scrolling.

garoto commented 1 year ago

Using the CI build posted by @xavery above (https://github.com/xavery/transgui/suites/13024903673/artifacts/705579187), the program crashes in about ~3 seconds once I minimize the program window, either to the (Windows) tray or justr a regular taskbar minimize.

I'm using Windows 8.1, but should it really matter? Odd if true.

Let me know if my transgui.ini or any other settings related to my Windows environment are needed.

Edit: https://x0.at/CS_H.mp4 (previous mp4 link replaced with a faststart one).

lighterowl commented 1 year ago

@garoto Thanks a lot. Looks like my gut feeling telling me not to make a new release right away was right this time...

I managed to reproduce the crash without even trying too hard. The only Windows-exclusive change which has been made in the last few weeks (both in my fork and the community project) was https://github.com/xavery/transgui/commit/76d3d6d8c956d03f021434191a11907c3ae1d398 and reverting it seems to make the crash go away.

Please try with the newest master build if you can : https://github.com/xavery/transgui/suites/13068302160/artifacts/708772515

garoto commented 1 year ago

@xavery It seems your supposition is indeed correct! The build you linked above is running crash-free while tray-minimized for the past ~25min. Yay?

btTeddy commented 1 year ago

Same. Here. Works just fine, haven't noticed any problems so far.

Thanks xavery

blacklion commented 1 year ago

I think fixing this is a big mistake. Current "official" GUI build always left current torrent selected and it is VERY useful in many situations.

For example: you have torrents sorted by size, but want to see torrents with names similar to THIS torrent. You select THIS torrent and sort by name - you see all similar names, as THIS torrent still selected!

To go to top of the list you don't need to "scroll up through all of the torrents" you need to press "Home" button on keyboard once (or "End" for last torrent in the list).

IMHO, old behavior is much, much better. Losing selection is no-no, IMHO.