-
-
Notifications
You must be signed in to change notification settings - Fork 792
Add support for column resizing in Qt tables #4189
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
base: main
Are you sure you want to change the base?
Add support for column resizing in Qt tables #4189
Conversation
johnzhou721
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, and welcome to BeeWare! (Disclaimer that I'm not an official core team member.)
My thoughts on this is that the implementation is looking great -- however -- could you explain how you came to the decision to stop preserving column widths once the user resizes? I'm just curious as to where you came to this decision; for me, I'd personally prefer preserving user proportions of columns when the layout changes -- but that's kind of bikesheddy, and may not be worthy of too much discussion
That said, some small notes inline.
| @@ -0,0 +1 @@ | |||
| Qt Table columns can now be resized by users, and manual column widths are preserved when the table source changes. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be 4109.misc.md because Qt table hasn't been in a relase yet.
| supports_icons = 2 # All columns | ||
| supports_keyboard_shortcuts = False | ||
| supports_widgets = False | ||
| supports_column_resize = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a flag isn't neccesary for this; implementing a probe method resize_column for all backends should be better, with pytest.skip representing platforms that hasn't implemented this yet.
(resize_column should be added and the test should be ran on all platforms; if we find that column resizing does not work, we could skip tests in this PR.)
| # A subsequent layout/bounds update should also preserve manual widths. | ||
| widget._impl.set_bounds(0, 0, probe.width + 40, probe.height) | ||
| await probe.redraw("Table bounds updated") | ||
| assert probe.column_width(0) == pytest.approx(resized_width, abs=8) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming we do go with the design you came up with, should we also test that not doing any resize will preserve proportions in layout?
| assert probe.column_width(0) == pytest.approx(resized_width, abs=8) | ||
|
|
||
| # A subsequent layout/bounds update should also preserve manual widths. | ||
| widget._impl.set_bounds(0, 0, probe.width + 40, probe.height) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is kind of fragile; we should do something like setting an explicit width as part of the layout specificaiton on widget, like widget.width = 400.
Summary
Interactivemode with a minimum section size so users can resize columnsCloses #4109.
Validation
uv run --python 3.12 --with ruff ruff check qt/src/toga_qt/widgets/table.py qt/tests_backend/widgets/table.py testbed/tests/widgets/test_table.pypython3 -m compileall qt/src/toga_qt/widgets/table.py qt/tests_backend/widgets/table.py testbed/tests/widgets/test_table.pyQT_QPA_PLATFORM=offscreen uv run --python 3.12 --with ./travertino --with ./core --with ./qt --with PySide6-Essentials~=6.10.0 pythonscript to verify:change_source()set_bounds()PR Checklist: