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

[luke-jr]

Regression introduced in #205

Correction: Undefined behaviour currently makes this a non-issue, but probably better to be safe and do it correctly.

[hebasto]

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]

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]

@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?

[luke-jr]

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]

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]

This discussion has been inactive for a long time.

Should it be closed?

[RandyMcMillan]

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]

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]

@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]

🤔 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]

Closing due to lack of activity.