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 778886 - Frame: property label-yalign does nothing, since the border node was removed
Frame: property label-yalign does nothing, since the border node was removed
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
3.89.x
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
https://gitlab.gnome.org/GNOME/gtk/me...
Depends on:
Blocks:
 
 
Reported: 2017-02-18 19:23 UTC by Daniel Boles
Modified: 2018-03-01 20:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Frame—Remove the now-useless property label-yalign (9.11 KB, patch)
2017-02-18 19:32 UTC, Daniel Boles
none Details | Review
[PATCH 2/2] Frame: Rename (get|set)_label_align() to _xalign() (4.17 KB, patch)
2017-02-25 00:49 UTC, Daniel Boles
none Details | Review
[PATCH 1/2] Frame: Erase the now-useless property label-yalign (9.10 KB, patch)
2017-02-25 00:59 UTC, Daniel Boles
accepted-commit_now Details | Review

Description Daniel Boles 2017-02-18 19:23:17 UTC
This used to control where the label-widget appeared relative to the border node. The default of 0.5 meant that the label-widget would get positioned vertically centered across the border (assuming the user didn't set flat style on the frame).

The border CSS node has been removed, as of commit 96a94fc21d0baf6972bb05038a2fd03dced8147d - with the result that the border is drawn around the entire Frame. So, now the label-widget is always located inside the top-left (assuming no overridden xalign) of the frame/border, and the label-yalign property has no purpose/effect.

It's hard to think of this property ever recovering any role, assuming the border node is staying gone... which I hope it doesn't :( But if that's not coming back, then the label-yalign property is dead wood and should be removed.
Comment 1 Daniel Boles 2017-02-18 19:32:49 UTC
Created attachment 346148 [details] [review]
Frame—Remove the now-useless property label-yalign

    The border is now drawn by the entire Frame, not as an internal border
    node, so we are no longer able to align the label to vertically overlap
    the border. The property no longer has any effect, so no use keeping it.
Comment 2 Daniel Boles 2017-02-25 00:49:31 UTC
Created attachment 346685 [details] [review]
[PATCH 2/2] Frame: Rename (get|set)_label_align() to _xalign()

Their API just changed by dropping the yalign arg, so while users are
adapting to that change, it is a free opportunity to unify the names of
the methods with the property they now solely reflect, label-xalign.
Comment 3 Daniel Boles 2017-02-25 00:50:33 UTC
Comment on attachment 346148 [details] [review]
Frame—Remove the now-useless property label-yalign

(tweaked the commit subject [no more pesky em dash] and message since this)
Comment 4 Daniel Boles 2017-02-25 00:59:25 UTC
Created attachment 346686 [details] [review]
[PATCH 1/2] Frame: Erase the now-useless property label-yalign

The border is now drawn on the frame node, not using an internal border
node, so we are no longer able to align the label to vertically overlap
the border. The property no longer performs its original purpose, & nor
is it a useful candidate for giving a new role, so no point keeping it.

updated commit message + fixed missing return value (return_val_if_fail)
Comment 5 Matthias Clasen 2018-02-10 22:50:29 UTC
Review of attachment 346686 [details] [review]:

sure, seems right for gtk4
Comment 6 Matthias Clasen 2018-02-18 03:08:53 UTC
Doesn't apply cleanly. Can you redo these and send an MR ?
Comment 7 Daniel Boles 2018-02-19 09:59:02 UTC
Yeah, I got the same result - have a rebase that will probably work but have been having jhbuild troubles (says that a build dir for meson 0.44 doesn't exist) and didn't have time to figure that out yet.
Comment 8 Daniel Boles 2018-03-01 19:59:48 UTC
(In reply to Matthias Clasen from comment #6)
> Doesn't apply cleanly. Can you redo these and send an MR ?

finally: https://gitlab.gnome.org/GNOME/gtk/merge_requests/40