Skip to content

Rework left side for better scrollbars and static buttonsArea#102

Merged
markotoplak merged 3 commits into
biolab:masterfrom
irgolic:control-area-scrollbars
Dec 17, 2020
Merged

Rework left side for better scrollbars and static buttonsArea#102
markotoplak merged 3 commits into
biolab:masterfrom
irgolic:control-area-scrollbars

Conversation

@irgolic

@irgolic irgolic commented Sep 27, 2020

Copy link
Copy Markdown
Member

Check this out with biolab/orange3#5013.

Issue

Current implementation shows/hides scrollbar each time it's shown, this is distracting.

Also, it'd be nice to have some buttons always visible in widgets (e.g., data projection controls), regardless of scroll position.

Screenshot 2020-10-02 at 23 48 00

Description of changes

Implements a solution similar to the one in biolab/orange-canvas-core#122, making it dynamically update the scroll area's width when scrollbars are disabled/enabled, and when the window is large enough vertically to not show the scrollbar. Therefore, the left side can always be encapsulated in a VerticalScrollArea, making left_side_scrolling obsolete.

Also, the buttonsArea was now docked below a scrollable controlsArea. This change deprecates buttons_area_orientation.

Includes
  • Code changes
  • Tests
  • Documentation

Comment thread orangewidget/widget.py Outdated
@irgolic
irgolic force-pushed the control-area-scrollbars branch 3 times, most recently from 3c83582 to b83c8c6 Compare September 28, 2020 18:51
@irgolic

irgolic commented Sep 28, 2020

Copy link
Copy Markdown
Member Author

I can't figure out why Heat Map in Orange3 behaves differently from other widgets. I think everything else is fine.

I'd like to make the left side a vertical box, with a scroll area on the top and buttonsArea on the bottom. I'll need some expert help with this, though, splitters and scroll areas are weird.

@irgolic
irgolic marked this pull request as draft October 1, 2020 10:38
@ales-erjavec

Copy link
Copy Markdown
Collaborator

I can't figure out why Heat Map in Orange3 behaves differently from other widgets.

Others do to (to a lesser extent). E.g. Scatter plot when extended horizontally to 2560px.
Screenshot 2020-10-02 at 13 02 03

(Heat Map has a too small sizeHint for the main area; that is why it is more pronounced there)

I think the problem is that QSplitter does not respect the QSizePolicy.Fixed size policy (QSplitter is really a bad fit for this use case here).

The left area would need to define a correct maximumSize (unfortunately this is not a virtual method/property) or explicitly set splitter.setSizes([...]) to 'unbalanced' sizes e.g. setSizes([1, QWIDGETSIZE_MAX]) in init (like it is done in __setControlAreaVisible)

@irgolic
irgolic force-pushed the control-area-scrollbars branch from b83c8c6 to 32a66aa Compare October 2, 2020 20:05
@irgolic
irgolic force-pushed the control-area-scrollbars branch from 32a66aa to 94cf345 Compare October 2, 2020 20:14
@irgolic irgolic changed the title Rework control area scrollbar adjustments Rework left side for better scrollbars and static buttonsArea Oct 2, 2020
@irgolic

irgolic commented Oct 2, 2020

Copy link
Copy Markdown
Member Author

@ales-erjavec That's genius, thank you so much. <3

I've gone ahead and reworked how the buttonsArea works too, adding a want_buttons_area option that defaults to False.

This is a bit of a vision not in the scope of this PR, but wouldn't it be nice if you didn't have to specify want_x options? Maybe the areas could automatically build themselves when they're first referenced as a parent?

At some point I'm going to go over the margins as they're inconsistent, the distance between two boxes should be equal to the controlsArea margin to the window frame.

@irgolic
irgolic marked this pull request as ready for review October 2, 2020 21:38
@irgolic
irgolic marked this pull request as draft October 4, 2020 15:03
@irgolic
irgolic force-pushed the control-area-scrollbars branch from 30ad623 to 020bf69 Compare October 4, 2020 16:23
@irgolic

irgolic commented Oct 4, 2020

Copy link
Copy Markdown
Member Author

Upon further inspection, I've removed want_buttons_area entirely. Now, if there's both a control area and main area, the control area will always have a buttons area below it.

@irgolic
irgolic marked this pull request as ready for review October 4, 2020 16:27
@irgolic irgolic mentioned this pull request Oct 5, 2020
@irgolic
irgolic force-pushed the control-area-scrollbars branch from 020bf69 to 71bda5a Compare November 3, 2020 22:42
@markotoplak

Copy link
Copy Markdown
Member

I have tested it both on Windows and MacOS. Seems to work nicely!

@markotoplak

Copy link
Copy Markdown
Member

@irgolic, I added a commit that makes scrolling possible also with widgets that lack the main area. For an example, check the Color widget. When the widget is extended, its behaviour should be the same as before, but now we can also make it smaller (then, scrollbars appear). Could you perhaps check if I unintentionally destroyed anything?

To make it work I needed to change a line in VerticalScrollArea; size hints are now computed from sizeHint().width() instead of width(). Without the fix widgets were only able to grow wide and could not be made narrower. :)

That commit does not change anything for widgets that have resizing_enabled = False. I thought about "overriding" that feature too, but maybe it is not a good idea. That settings also sets the window type: on a linux machine with a tiling window manager resizing_enabled = False widgets popup as dialogs (floating windows), while others are expanded as much as possible (tiled windows). So for very small widgets it really does make sense.

An idea, for some other PR, perhaps: if widget's sizeHint is bigger than some limit, forcefully behave as if resizing_enabled=True. And show a warning. What do you think, @irgolic?

@irgolic

irgolic commented Nov 7, 2020

Copy link
Copy Markdown
Member Author

I agree that there's no need to remove resizing_enabled. Our energy's better directed elsewhere than optimizing that one away.

I like the idea of showing scrollBars in widgets without mainAreas, but I don't like how there's a scrollbar visible by default.

Screenshot 2020-11-07 at 16 14 50

What if, given no mainArea, we set it to minimumExpanding, and clamp the sizeHint height of the scroll_area? That way, if the controlArea is above a certain height, it'll appear with a scrollbar, but won't let you resize it below that clamped height, and won't let you do this with windows:

Screenshot 2020-11-07 at 16 15 42

With a change like this, this PR will make testing widget height obsolete. I suppose width should still be tested?

@markotoplak

markotoplak commented Nov 7, 2020

Copy link
Copy Markdown
Member

Well, good that you tried it out, I did not get any scrollbars by default on my computer. That is what makes these things fun!

I did not quite understand how you'd solve this one, but I do agree with your proposal. Makes sense not to be able to make it all too small. If you have an idea how to do it, just proceed.

Then, we could probably forget about width testing. I think it only came up as an issue once.

@markotoplak
markotoplak force-pushed the control-area-scrollbars branch from 2f19447 to 929094a Compare November 19, 2020 16:01
@irgolic
irgolic force-pushed the control-area-scrollbars branch 2 times, most recently from 8a83713 to e6ad112 Compare November 19, 2020 17:18
- Adjust VerticalScrollArea for dynamic transient scrollbar adjustments
- Put buttonsArea below scrolling controlArea
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants