-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Qt issue 2862 last column cannot be resized #3626
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
Qt issue 2862 last column cannot be resized #3626
Conversation
|
Issue #2862 was about the old-receive-tab. Actually you can resize the last column in the new receive tab, but you have to move your mouse to the right end of the column. I wouldnt expect it there either, but it works for me. In the addressbookpage.cpp, we have set ResizeToContents on the last column, to ensure its contents are always visible. I think this might be a good idea for the new receive-tab and also the transactions-tab too. So unless there is any other very important reason, why someone would want to resize the last column, I would rather say we consider this as an upstream qt problem. |
|
" Actually you can resize the last column in the new receive tab, but you have to move your mouse to the right end of the column." yes, I noticed this when I made the first change, I've put all these in comments I believe. You can now grab in between the last two columns an resize from there. It responds also to window resizing, etc. If you build this branch you'll see that it works flawlessly now, as you'd expect. I also thought of ResizeToContents but that would've been too easy (and wouldn't reflect what the user might want, perhaps they want that last column really big for some reason) I want to do it for the transactions tab as well and see if there's a pattern I can abstract, so I can then refactor from an abstraction of either QTableWidget or QHeaderView that we can reuse everywhere. |
|
@gubatron This indeed solves the issue but at the cost of micromanaging every aspect of Qt sizing. This adds a lot of low-level code to the dialog for such a minor change. Can it be generalized and moved to (a class in) guiutils somehow? @cozz Please don't use resizeToContents on the transactions tab or any other table that may grow big. As Qt has to look at every single row in the model to decide how to size the column; it doesn't scale. |
|
@laanwj fair enough. I'llI do that. Thanks for pointing me in the right direction as I'm very new to the codebase. Once I've been able to generalize a solution and put it in guiutils I'll apply to all the tables I see have the same issue (if you don't mind). |
|
(working on generic solution today, nothing to look at here, just did a merge from the last changes in the main branch) |
…apsulates low level resizing calculations and it is now reusable for other tables.
|
Made the fix an utility class in GUIUtils as suggested by @laanwj , applied it on both tables, and I believe it's working flawlessly now, it also adapts the size of the columns well in case of a windows resize event, won't let the table columns get bigger than the table viewport either. git help to merge these commits into a single patch.I might need some help with the rebase-squash to make this into a single commit, help would be appreciated (I was playing with it in another project and I ended up squashing other commits I had merged from master into my feature branch) As I was developing this I think I made the mistake of merging changes from the bitcoin master banch into my feature branch as I was working on it (was that a mistake if I then intended to squash all my commits as a single one?) My first revision for this pull request was a70df08 |
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/0a95ec59c0cfddb26aa4e63847da6d2a22f2e06f for binaries and test log. |
|
Squashing with merges in between is non-trivial. It is possible but I don't know by heart. Ideally, you do But it's too late for that now :) What you could do is create one commit from the diff between your feature branch and the last master branch that you merged. This is basically a sledge-hammer approach but it should work. And you keep your old branch as After this you could rebase to the new master. Don't do any merges :) |
|
closing this pull request, sending a new one that has a single commit with all the changes. Thank you so much guys. |
…er class 3913d1e qt: Drop buggy TableViewLastColumnResizingFixer class (Hennadii Stepanov) Pull request description: In Qt 5 the last column resizing with dragging its left edge works out-of-the-box. The current `TableViewLastColumnResizingFixer` implementation could put the last column content out of the view port and confuse a user:  Historical context: - #2862 - #3626 - #3738 - #3920 #205 is a nice addition. ACKs for top commit: jarolrod: ACK 3913d1e, tested on macOS 11.1 Qt 5.15.2 Talkless: tACK 3913d1e, tested on Debian Sid. Can confirm that behavior in previous commit does not produce scroll bar, last column gets "hidden". This PR makes clear that there's more to see in the view. promag: Tested ACK 3913d1e on macos. Tree-SHA512: 12582dfce54bb1db3d9934ae092e305d32e9760cc99b0265322e161fa7f54b7d6fb6cefedf700783f767d5c3a56a8545c8d2f5ade66596c4e67b8a5287063e8a
…er class 3913d1e qt: Drop buggy TableViewLastColumnResizingFixer class (Hennadii Stepanov) Pull request description: In Qt 5 the last column resizing with dragging its left edge works out-of-the-box. The current `TableViewLastColumnResizingFixer` implementation could put the last column content out of the view port and confuse a user:  Historical context: - bitcoin#2862 - bitcoin#3626 - bitcoin#3738 - bitcoin#3920 #205 is a nice addition. ACKs for top commit: jarolrod: ACK 3913d1e, tested on macOS 11.1 Qt 5.15.2 Talkless: tACK 3913d1e, tested on Debian Sid. Can confirm that behavior in previous commit does not produce scroll bar, last column gets "hidden". This PR makes clear that there's more to see in the view. promag: Tested ACK 3913d1e on macos. Tree-SHA512: 12582dfce54bb1db3d9934ae092e305d32e9760cc99b0265322e161fa7f54b7d6fb6cefedf700783f767d5c3a56a8545c8d2f5ade66596c4e67b8a5287063e8a
Fixes issue with the Receive table in which users could not resize the "Amount" column.
Performed a few cleanups and refactors along the way for readability and avoidance of code-repetition.