-
-
Notifications
You must be signed in to change notification settings - Fork 80
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
Simple syntax changes #2553
base: jeremypw/gtk4-skeleton
Are you sure you want to change the base?
Simple syntax changes #2553
Conversation
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.
Mostly straightforward, just a few comments
Co-authored-by: Danielle Foré <danielle@elementary.io>
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.
You resolved the one with removing show_all
where it should be present
but there's a bunch of other ones in this diff where a window or dialog is using show_all
and if you just remove it then the window will never be shown
Sorry - I overlooked dialogs need to be shown whereas other widgets you can just delete |
I have pushed another PR applying the dialog fixes to the main branch where possible. |
var grid = new Gtk.Box (VERTICAL, 0); | ||
|
||
grid.append (header); | ||
grid.append (new Gtk.Separator (Gtk.Orientation.HORIZONTAL)); | ||
grid.append (chooser); | ||
grid.append (new Gtk.Separator (Gtk.Orientation.HORIZONTAL)); | ||
grid.append (choices_box); | ||
// grid.append (action_box); | ||
child = grid; |
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.
While we're here, can we rename this to box
since it isn't a grid anymore :p
@@ -11,22 +11,19 @@ public class PortalTester : Gtk.Application { | |||
window.set_default_size (400, 400); | |||
window.title = "Files Portal Tester"; | |||
|
|||
var grid = new Gtk.Grid () { | |||
var grid = new Gtk.Box (VERTICAL, 6) { |
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.
We removed Portaltester in another branch so I guess this needs to be rebased?
// content_box.pack_start (overlay, true, true, 0); | ||
// overlay.add (widget); | ||
overlay = new Gtk.Overlay () { | ||
child = widget |
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.
pack_start
had true
for expand so I think we need to add hexpand = true
and vexpand = true
|
||
sidebar = new Sidebar.SidebarWindow (); | ||
free_space_change.connect (sidebar.on_free_space_change); | ||
|
||
lside_pane = new Gtk.Paned (Gtk.Orientation.HORIZONTAL) { | ||
// expand = true, | ||
hexpand = 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.
vexpand also maybe?
Assorted simple changes mainly to accomodate changes in syntax. These changes should not have any functional effect.