Skip to content
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

Drop buggy TableViewLastColumnResizingFixer class #204

Merged
merged 1 commit into from
Feb 22, 2021

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Jan 31, 2021

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:
Screenshot from 2021-01-31 18-04-32

Historical context:

#205 is a nice addition.

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.
Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 3913d1e, tested on macOS 11.1 Qt 5.15.2

On master, when you resize, the last column can go out of the view-port. This can be confusing because there's no contextual clues that there is more to see by scrolling to the right. This PR add this contextual awareness by showing a scrollbar. Below are screenshots showing this behavior:

Master: Last column can go out of view port while resizing
Screen Shot 2021-02-08 at 3 05 33 AM

PR: scrollbar gives contextual awareness that I can scroll right to see more
Screen Shot 2021-02-08 at 3 03 37 AM

@hebasto
Copy link
Member Author

hebasto commented Feb 21, 2021

@promag @jonatack Do you mind reviewing this PR?

Copy link

@Talkless Talkless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested ACK 3913d1e on macos.

tableView->setModel(_model->getRecentRequestsTableModel());
tableView->setAlternatingRowColors(true);
tableView->setSelectionBehavior(QAbstractItemView::SelectRows);
tableView->setSelectionMode(QAbstractItemView::ContiguousSelection);
tableView->setColumnWidth(RecentRequestsTableModel::Date, DATE_COLUMN_WIDTH);
tableView->setColumnWidth(RecentRequestsTableModel::Label, LABEL_COLUMN_WIDTH);
tableView->setColumnWidth(RecentRequestsTableModel::Amount, AMOUNT_MINIMUM_COLUMN_WIDTH);
tableView->horizontalHeader()->setMinimumSectionSize(MINIMUM_COLUMN_WIDTH);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed this and didn't notice differences, at least on macos.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It prevents
Screenshot from 2021-02-21 21-41-20

at least on Linux :)

@maflcko maflcko merged commit fd725c2 into bitcoin-core:master Feb 22, 2021
maflcko pushed a commit that referenced this pull request Feb 22, 2021
…s column sizes

964885d qt: Save/restore recentRequestsView table column sizes (Hennadii Stepanov)
f5c8093 qt: Move recentRequestsView properties settings to constructor (Hennadii Stepanov)
9c5f4f2 qt: Save/restore TransactionView table column sizes (Hennadii Stepanov)
788205c qt: Move transactionView properties settings to constructor (Hennadii Stepanov)
ecdbaf7 qt, refactor: Drop intermediate assignment (Hennadii Stepanov)

Pull request description:

  Save/restore TransactionView and recentRequestsView tables column sizes.
  Sorting order is not saved/restored intentionally.

  Based on #204 (the first commit).

ACKs for top commit:
  jarolrod:
    ACK 964885d, tested on macOS 11.1 Qt 5.15.2
  Talkless:
    tACK 964885d, tested on Debian Sid, saving/restoring and resetting (with `-resetguisettings`) works as expected.

Tree-SHA512: c24e41bf4d95bb33dce16e9a0b952ffd0912e95f4d2a1bc5292fcf5a27100e70fea73433c4ff246d05b174fc23a7b6de1790a2e8b990a9089e4deca79a00dedc
@hebasto hebasto deleted the 210131-resize branch February 22, 2021 07:20
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 22, 2021
…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:
  ![Screenshot from 2021-01-31 18-04-32](https://user-images.githubusercontent.com/32963518/106390022-fd6bd180-63ee-11eb-9216-6e5117f8dc96.png)

  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
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 28, 2022
Some GUI changes that conflict with commented-out code. I made an educated
guess as to what new stuff should be commented out or not but honestly I
have no idea what the GUI is supposed to do or show.
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants