Widget sizeHint, button/box sizePolicy fixes by irgolic · Pull Request #130 · biolab/orange-widget-base

irgolic · 2021-02-05T18:13:12Z

Issue

Widgets aren't visually consistent. It'll take a little bit of work in Orange3 and add-ons, but once we're there, everything will be nice. Most of the changes I made in the Orange3 PR simplify the code, I hope that's the case for addons too.

Description of changes

See commit history.

This was written on macOS 10.15.7, working with both QMacStyle and Plastic. Hoping Windows and Linux look ok.

Includes

Code changes
Tests
Documentation

ajdapretnar · 2021-02-09T09:30:01Z

I don't think Time Slice has this option anymore.

irgolic · 2021-02-12T10:47:33Z

-        else:
-            separator(widget)
+    warnings.warn("'addSpace' has been deprecated. Use gui.separator instead.",
+                  DeprecationWarning, stacklevel=3)


Better deprecate (someone test this pls I might be doing it wrong)

markotoplak · 2021-02-15T08:38:44Z

While I do agree that this branch combined with the PR in Orange3 makes Orange look nicer, I can not merge this. For me, there is too much new additional magic in miscellanea (allowing box_ and parent_ settings) and complicated and non-avoidable magical widget sizeHint.

Mac-specific changes also bother me somewhat, but if that is really needed to fix strange alignments, well, then fine. These I could actually merge...

@ales-erjavec, do you have any time to review this? If you say these code patterns are fine, then I'll accept them.

markotoplak · 2021-02-15T08:45:14Z

I can merge this PR if:

There is an option to disable sizeHint.
The new box_, parent_, multiple _ magic gets removed from miscelanea. Yes, sometimes we will have to use multiple lines of code instead of additional parameters... But if we could survive without these before, they do not seem to be needed often.

ales-erjavec · 2021-02-15T10:43:59Z

+    if button.style().metaObject().className() == 'QMacStyle':
+        btnpaddingbox = vBox(widget, margin=0, spacing=0)
+        separator(btnpaddingbox, 0, 4)  # lines up with a WA_LayoutUsesWidgetRect checkbox
+        button.outer_box = btnpaddingbox


Why would you assume anything related to checkbox here?

Autocommit is a button next to a checkbox. Generally you want all buttons in a row (QHBoxLayout) at the same height, so it's set globally.

Without this, still keeping Qt.WA_LayoutUsesWidgetRect will misalign the checkbox and button.

ales-erjavec · 2021-02-15T12:42:29Z

@ales-erjavec, do you have any time to review this? If you say these code patterns are fine, then I'll accept them.

I don't like it.

I already cannot reason about what the existing code does. This would make it even worse.
But then again I advise against using gui at all and if this fixes existing uses (for Orange only?) then go ahead.

If you do remove addSpace (good) why then add addSpaceBefore (bad)?

I would keep some changes that fix layouts locally, I.e. the one that removes near global WA_LayoutUsesWidgetRect and moves it to the function where it is required, margins/spacings in created widgets/layouts, ...

ales-erjavec · 2021-02-15T12:46:18Z

+    if box and parent and box is not parent and parent.layout() and parent.layout().spacing() == -1:
+        parent.layout().setSpacing(8)


Remove this. If you keep it add addToLayout to the if condition. But really remove this.

MacStyle is weird. It doesn't use constant spacing, it uses this weird matrix to calculate spacing between two controls, depending on which one precedes the other. https://doc.qt.io/archives/qq/qq23-layouts.html

This works great if you only use control elements. But as soon as you nest controls in boxes (e.g., label + combo), the margins and spacing get really big. Our solution for this is to set the spacing on the box (if no spacing has yet been set), if you add a nested control. For this to work right, Qt.WA_LayoutUsesWidgetRect should be set on all the controls in the parent box.

... That was our rationale, but I've removed it, and I think we'll live without it.

ales-erjavec · 2021-02-15T14:39:28Z

+            if widget and widget.layout() and \
+                    isinstance(widget.layout(), QtWidgets.QVBoxLayout) and \
+                    not widget.layout().isEmpty():
+                misc.setdefault('addSpaceBefore', True)


Do not add back another automagic to replace the addSpace one.

addSpaceBefore replaced addSpace, it's there to put separators between boxes, but not before the first box. I could make it private to the gui package, then you could disable it with addToLayout=False.

Initially this helped align checkboxes with checkboxes in their nested boxes (e.g. with a spinbox). But, this messes with MacStyle spacing, and makes everything a bit too spaced out. If this is necessary, put all controls in their own box, or set the attribute manually.

Also fixup some spacing/margins

irgolic changed the title ~~gui: Set maximum autocommit btn height~~ gui: Keep Macstyle autocommit, allow boxes to expand Feb 5, 2021

irgolic mentioned this pull request Feb 8, 2021

Use buttonsArea, UI cleanup biolab/orange3#5013

Merged

3 tasks

irgolic changed the title ~~gui: Keep Macstyle autocommit, allow boxes to expand~~ Widget sizeHint, button/box sizePolicy fixes, rubber Feb 8, 2021

irgolic changed the title ~~Widget sizeHint, button/box sizePolicy fixes, rubber~~ Widget sizeHint, button/box sizePolicy fixes Feb 8, 2021

irgolic force-pushed the keep-button-height branch from 71c7508 to 8ab8f1d Compare February 10, 2021 19:05

irgolic mentioned this pull request Feb 11, 2021

Big margin above the apply button biolab/orange3#5174

Closed

3 tasks

irgolic commented Feb 12, 2021

View reviewed changes

irgolic force-pushed the keep-button-height branch 4 times, most recently from 3408551 to b328515 Compare February 13, 2021 23:27

markotoplak reviewed Feb 15, 2021

View reviewed changes

Comment thread orangewidget/widget.py

markotoplak reviewed Feb 15, 2021

View reviewed changes

Comment thread orangewidget/widget.py

markotoplak reviewed Feb 15, 2021

View reviewed changes

Comment thread orangewidget/gui.py Outdated

markotoplak reviewed Feb 15, 2021

View reviewed changes

Comment thread orangewidget/gui.py Outdated

ales-erjavec reviewed Feb 15, 2021

View reviewed changes

irgolic added 9 commits February 17, 2021 21:25

gui: Allow autocommit box to expand

4e6167c

widget: Allow controlArea to expand

6621623

widget: Set correct buttonsArea margin

06193f0

widget: Set controlArea vertical sizePolicy to Preferred

7628ba8

widget: Remove main area/graph coupling

fe76121

widget: Define sizeHint

d3c1dec

gui: Deprecate addSpace, replace with spacing

f01970a

Also fixup some spacing/margins

gui: Unsplat miscellaneous tuple args

671597e

irgolic added 6 commits February 17, 2021 21:25

gui: Make widgetbox spacing optional

5c32e63

gui: addSpaceBefore boxes with titles

79b7942

gui: Remove auto_commit box, space better

47d8986

gui: set sizePolicy kwarg to both box and control

edc9e5d

gui: Deprecate addSpace=False too

eda2172

gui: createTabPage orientation kwarg

420aa1f

irgolic force-pushed the keep-button-height branch from b328515 to f10329c Compare February 18, 2021 01:32

irgolic added 6 commits February 18, 2021 01:41

gui: Add Macstyle specific styling

c12a384

gui: Refurbish autocommit cb margins

132977c

widget: Give buttonsArea some left/right margin

9797c77

widget: Expose mainArea_width_height_ratio

134ea74

gui: Set autocommit vertical sp to Maximum

ba89e7e

widget: Add separator between areas if no main

f01b90b

irgolic force-pushed the keep-button-height branch from f10329c to f01b90b Compare February 18, 2021 01:41

janezd mentioned this pull request Feb 19, 2021

Widget UI changes following new orange-widget-base release biolab/orange3-network#200

Closed

markotoplak merged commit bc6f08c into biolab:master Feb 19, 2021

		if box and parent and box is not parent and parent.layout() and parent.layout().spacing() == -1:
		parent.layout().setSpacing(8)

Uh oh!

Conversation

irgolic commented Feb 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Description of changes

Includes

Uh oh!

ajdapretnar commented Feb 9, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

irgolic Feb 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

markotoplak commented Feb 15, 2021

Uh oh!

markotoplak commented Feb 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ales-erjavec Feb 15, 2021

Choose a reason for hiding this comment

Uh oh!

irgolic Feb 18, 2021

Choose a reason for hiding this comment

Uh oh!

ales-erjavec commented Feb 15, 2021

Uh oh!

ales-erjavec Feb 15, 2021

Choose a reason for hiding this comment

Uh oh!

irgolic Feb 18, 2021

Choose a reason for hiding this comment

Uh oh!

ales-erjavec Feb 15, 2021

Choose a reason for hiding this comment

Uh oh!

irgolic Feb 18, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

irgolic commented Feb 5, 2021 •

edited

Loading

irgolic Feb 12, 2021 •

edited

Loading

markotoplak commented Feb 15, 2021 •

edited

Loading