After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 703808 - Remove support for fixed positioning in BoxLayout
Remove support for fixed positioning in BoxLayout
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks: 703810
 
 
Reported: 2013-07-08 17:48 UTC by Florian Müllner
Modified: 2015-05-13 15:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
layout: Use a custom layoutManager for panelBox (3.40 KB, patch)
2013-07-08 17:48 UTC, Florian Müllner
needs-work Details | Review
overview: Add coverPane to stack instead of BoxLayout (1.21 KB, patch)
2013-07-08 17:48 UTC, Florian Müllner
committed Details | Review
st: Remove support for fixed positioning in BoxLayout (4.71 KB, patch)
2013-07-08 17:48 UTC, Florian Müllner
committed Details | Review
lookingGlass: Don't use fixed positions in BoxLayout (5.47 KB, patch)
2013-07-08 21:21 UTC, Florian Müllner
none Details | Review
lookingGlass: Use uiGroup as parent instead of panelBox (2.02 KB, patch)
2013-07-08 21:21 UTC, Florian Müllner
committed Details | Review

Description Florian Müllner 2013-07-08 17:48:32 UTC
It is odd API, undocumented, hardly used and in the way of some major cleanup - kill it.
Comment 1 Florian Müllner 2013-07-08 17:48:37 UTC
Created attachment 248634 [details] [review]
layout: Use a custom layoutManager for panelBox

Currently lookingGlass relies on some odd BoxLayout behavior, which
allows children to use fixed positioning without affecting the parent's
size request. As this behavior is scheduled for removal, use a custom
layout manager to achieve the desired behavior instead.
Comment 2 Florian Müllner 2013-07-08 17:48:42 UTC
Created attachment 248635 [details] [review]
overview: Add coverPane to stack instead of BoxLayout

The event catcher that covers the entire primary monitor during
transitions is currently inside a BoxLayout, relying in its
odd support for fixed position actors.
We already have a proper stack widget in place, move it there.
Comment 3 Florian Müllner 2013-07-08 17:48:46 UTC
Created attachment 248636 [details] [review]
st: Remove support for fixed positioning in BoxLayout

It is the job of layout containers to arrange their children; having
a hidden feature that *also* allows children to be positioned freely
outside the parent's allocation is just odd.
With the last user of the feature gone, kill it.
Comment 4 Jasper St. Pierre (not reading bugmail) 2013-07-08 18:01:13 UTC
Review of attachment 248634 [details] [review]:

Why not remove it from the panelBox and remove the allocation-changed, etc. signals?
Comment 5 Jasper St. Pierre (not reading bugmail) 2013-07-08 18:01:35 UTC
Review of attachment 248635 [details] [review]:

OK.
Comment 6 Jasper St. Pierre (not reading bugmail) 2013-07-08 18:02:56 UTC
Review of attachment 248636 [details] [review]:

Yeah. Have you also considered removing pack-start?
Comment 7 Florian Müllner 2013-07-08 18:06:06 UTC
(In reply to comment #6)
> Review of attachment 248636 [details] [review]:
> 
> Yeah. Have you also considered removing pack-start?

Not really. The goal is to make StBoxLayout use a ClutterBoxLayout internally, so supporting pack-start will be dead-cheap (e.g. just passing on the property). We can still remove it later and make consumers use box.layout_manager.pack_start instead though ...
Comment 8 Jasper St. Pierre (not reading bugmail) 2013-07-08 18:15:45 UTC
There are no consumers of pack_start...
Comment 9 Florian Müllner 2013-07-08 18:26:39 UTC
Right, I meant "people who may want to use it in the future". It would be easier for me to leave it in until the ClutterBoxLayout patches have landed,  but if it makes you happy, I can rebase the set again ...
Comment 10 Jasper St. Pierre (not reading bugmail) 2013-07-08 18:30:21 UTC
Ah, I didn't see that pack_start was supported by ClutterBoxLayout as well..
Comment 11 Florian Müllner 2013-07-08 21:20:01 UTC
(In reply to comment #4)
> Why not remove [...] the allocation-changed, etc. signals?

Uh? We still need the panel size/position for sizing/positioning the dialog. Anyway, I'll attach two alternative patches which leave panelBox alone ...
Comment 12 Florian Müllner 2013-07-08 21:21:16 UTC
Created attachment 248667 [details] [review]
lookingGlass: Don't use fixed positions in BoxLayout

That's the fancier variant, which moves the trickery into lookingGlass itself.
Comment 13 Florian Müllner 2013-07-08 21:21:58 UTC
Created attachment 248668 [details] [review]
lookingGlass: Use uiGroup as parent instead of panelBox

The obvious alternative, move lookingGlass to uiGroup instead.
Comment 14 Jasper St. Pierre (not reading bugmail) 2013-07-08 21:44:38 UTC
Review of attachment 248668 [details] [review]:

Yeah, this is something more like what I meant.

Though I was going to suggest using a layout manager to position the looking glass globally, and continue to use translation_y for the tween.
Comment 15 Florian Müllner 2013-07-10 15:41:06 UTC
Attachment 248635 [details] pushed as 70da558 - overview: Add coverPane to stack instead of BoxLayout
Attachment 248636 [details] pushed as 53d268a - st: Remove support for fixed positioning in BoxLayout
Attachment 248668 [details] pushed as 1121537 - lookingGlass: Use uiGroup as parent instead of panelBox
Comment 16 sancelot 2015-05-13 14:58:52 UTC
Hi,
If you removed absolute positioning, how to set absolute positioning now !???

Regards,
Comment 17 Florian Müllner 2015-05-13 15:22:24 UTC
Bugzilla is not a support forum, and doubly so in case of old and closed bugs, but here you go:
Don't use StBoxLayout, use a container that uses a ClutterFixedLayout instead, like ClutterActor or StWidget.