Bugfix: When restoring table columns, still set their minimum column width and stretch on last section #368

pull luke-jr wants to merge 1 commits into bitcoin-core:master from luke-jr:bugfix_gui_restored_columns_stretch changing 2 files +4 −4

luke-jr commented at 2:19 am on June 24, 2021: member

~~Regression introduced in #205~~

Correction: Undefined behaviour currently makes this a non-issue, but probably better to be safe and do it correctly.
Bugfix: GUI: When restoring table columns, still set their minimum column width and stretch on last section 4a5a74e709
hebasto added the label Bug on Jun 24, 2021
hebasto added the label UI on Jun 24, 2021
hebasto renamed this:
~~Bugfix: GUI: When restoring table columns, still set their minimum column width and stretch on last section~~
Bugfix: When restoring table columns, still set their minimum column width and stretch on last section
on Jun 24, 2021
bitcoin-core deleted a comment on Jun 24, 2021
hebasto commented at 4:03 am on June 29, 2021: member

When restoring table columns, still set their minimum column width and stretch on last section

Are minimumSectionSize and stretchLastSection parts of the saved state?

Regression introduced in #205

How is it observable by a user?
jarolrod commented at 1:38 am on July 1, 2021: member
Are minimumSectionSize and stretchLastSection parts of the saved state?

yes, they both are. See: https://code.qt.io/cgit/qt/qt.git/tree/src/gui/itemviews/qheaderview.cpp#n3556
- minimumSectionSize
- stretchLastSection
Regression introduced in #205

While I think this change is ok, is it possible for a user to actually manipulate the header view in a way where they can run into an issue here?
hebasto commented at 9:01 am on August 8, 2021: member
@luke-jr

The minimumSectionSize and stretchLastSection properties of the QHeaderView class are saved with QHeaderView::saveState and restored with QHeaderView::restoreState.

Regression introduced in #205
1. From the PR description it is not clear what is actually a bug in the current master branch. Could you elaborate please?
2. What is the reason to set minimumSectionSize and stretchLastSection before restoring the header state, when during this restoring (if it succeeds, of course) the values of these properties will be overwritten?
hebasto added the label Waiting for author on Aug 8, 2021
luke-jr commented at 1:08 am on September 8, 2021: member

I guess this is more about not relying on undefined behaviour rather than a bugfix.

Leaving the assignment prior to restoring sizes, so that users who actually go to the trouble to hack their setting are respected.
hebasto removed the label Waiting for author on Sep 9, 2021
hebasto commented at 3:13 pm on September 9, 2021: member

I guess this is more about not relying on undefined behaviour rather than a bugfix.

Leaving the assignment prior to restoring sizes, so that users who actually go to the trouble to hack their setting are respected.

Why only are setMinimumSectionSize and setStretchLastSection considered? And not setColumnWidth methods?

And -resetguisettings could always help :)
hebasto commented at 10:34 pm on June 1, 2022: member

This discussion has been inactive for a long time.

Should it be closed?
RandyMcMillan commented at 1:52 pm on June 2, 2022: contributor

Let’s leave this open - In previous work I added a “bumper” column to the tableview - this helped address some of the issues. The bumper column was an addon to other PRs but not a stand alone PR - I will put together a PR that focuses on why this is useful.
DrahtBot commented at 9:49 pm on June 12, 2022: contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:
- #684 (Improve ‘Requested Payments History’ Multiselect by john-moffett)
- #662 (Fix transaction view/table by luke-jr)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
hebasto commented at 3:53 pm on July 21, 2022: member

@luke-jr

This discussion has been inactive for a long time.

Should it be closed?

Instead of reacting with the “thumbs down” emoji, maybe just answer questions from the comments?
DrahtBot cross-referenced this on Sep 6, 2022 from issue Fix transaction view/table by luke-jr
DrahtBot cross-referenced this on Dec 6, 2022 from issue Improve 'Requested Payments History' Multiselect by john-moffett
DrahtBot added the label CI failed on Aug 31, 2023
DrahtBot removed the label CI failed on Sep 4, 2023
DrahtBot added the label CI failed on Oct 21, 2023
DrahtBot commented at 2:34 pm on February 5, 2024: contributor

🤔 There hasn’t been much activity lately and the CI seems to be failing.

If no one reviewed the current pull request by commit hash, a rebase can be considered. While the CI failure may be a false positive, the CI hasn’t been running for some time, so there may be a real issue hiding as well. A rebase triggers the latest CI and makes sure that no silent merge conflicts have snuck in.
hebasto commented at 6:12 pm on February 7, 2024: member

Closing due to lack of activity.
hebasto closed this on Feb 7, 2024

Contributors
luke-jr hebasto jarolrod RandyMcMillan DrahtBot

Labels
Bug UI CI failed

Bugfix: When restoring table columns, still set their minimum column width and stretch on last section #368

Conflicts