Gtk4 port #877
Gtk4 port #877
Conversation
There was a problem hiding this comment.
Hell yeah, getting a lot closer! Left some comments as I was going through.
Is there any way we could land the new accounts list in the Gtk3 version or are we relying on features in Gtk4?
Unfortunately we're stuck with Gtk 4.6.6 until we rebase on Ubuntu 24.04 so we either have to wait until then to land the Gtk 4 port or stick with what is available in that version
| public signal void finished (); | ||
|
|
||
| private const string ACTION_GROUP_PREFIX = "win"; | ||
| private const string ACTION_PREFIX = ACTION_GROUP_PREFIX + "."; | ||
|
|
||
| private const string ACTION_ADD_ATTACHMENT= "add-attachment"; | ||
| private const string ACTION_ADD_ATTACHMENT= "append-attachment"; |
There was a problem hiding this comment.
Looks like a search/replace error 😅
| @@ -18,17 +18,14 @@ | |||
| * Authored by: Corentin Noël <corentin@elementary.io> | |||
| */ | |||
|
|
|||
| public class Mail.MainWindow : Hdy.ApplicationWindow { | |||
| public class Mail.MainWindow : Adw.ApplicationWindow { | |||
There was a problem hiding this comment.
Is there a reason for this to be an Adw.ApplicationWindow instead of Gtk.ApplicationWindow?
There was a problem hiding this comment.
Adw.ApplicationWindow prevents the window manager (or is it gtk that does it? 😅) from adding the standard title bar. I am not sure whether that's the only way to do it but I don't think set_titlebar would work because we've got multiple HeaderBars.
There was a problem hiding this comment.
Ah okay yeah, so the way we've been dealing with this in other places is set_titlebar (new Gtk.Grid () { visible = false });. I'm not sure if using Adw.ApplicationWindow will have any unintended consequences on styling etc
| to_grid.add (to_val); | ||
| to_grid.add (cc_button); | ||
| to_grid.add (bcc_button); | ||
| var to_box = new EntryBox (); |
There was a problem hiding this comment.
I think we don't need this since in Gtk 4 we can parent these buttons into the entry itself
| main_box.add (attachment_box); | ||
| main_box.add (action_bar); | ||
|
|
||
| // main_box.append (headerbar); |
There was a problem hiding this comment.
probably can remove this comment right? :)
|
|
||
| unowned var attachment_obj = (Attachment)attachment; | ||
| body.add_part (attachment_obj.get_mime_part ()); | ||
| Gtk.Widget current_attachment = attachment_box.get_first_child (); |
| box.append (size_label); | ||
| box.append (remove_button); | ||
|
|
||
| margin_top = 3; |
There was a problem hiding this comment.
Good call moving margin here
| // insert_button.can_default = true; | ||
| // insert_button.has_default = true; |
There was a problem hiding this comment.
This is now Gtk.Window.set_default_widget ()
| paned_start = new Gtk.Paned (Gtk.Orientation.HORIZONTAL) { | ||
| shrink_start_child = false, | ||
| shrink_end_child = false, | ||
| wide_handle = true |
There was a problem hiding this comment.
Is there a reason why you added the wide_handle here?
There was a problem hiding this comment.
I hoped this would fix some style issues with the separators because the black border on the left side of the conversation list is shorter than on the right side but it didn't and I'm not even sure whether the black borders belongs to the separators or to something else entirely. Any idea about the origin of these borders and how to fix their size?
|
|
||
| account_settings_menuitem.clicked.connect (() => { | ||
| try { | ||
| AppInfo.launch_default_for_uri ("settings://accounts/online", null); |
There was a problem hiding this comment.
We want to use Gtk.show_uri () here to avoid focus stealing prevention
| @@ -20,10 +20,8 @@ | |||
|
|
|||
| public class Mail.WelcomeView : Gtk.Box { | |||
There was a problem hiding this comment.
I think most of this can be replaced with Granite.Placeholder
It relies on
If the current look is ok we can stick with Gtk 4.6.6. Maybe a stupid question but does that mean we are stuck with 22.04 packages entirely? Or is there another reason for the gtk 4 restriction? |
|
Yeah exactly we have to ship Mail in deb currently because of issues with accessing the host keychain with evolution and it not working with online accounts. So we're stuck with Ubuntu's packages there |
|
I guess we'll have to wait for 24.04 then anyway. In the meantime is there anything I can help with getting the flatpak version to work? I've come over this, is it still relevant? |
|
I think @tintou probably knows the most about this since he's done upstream work with EDS but yeah my understanding is that the Flatpak EDS can't access the credentials stored in the host EDS |
As far as I know most features from the gtk3 version are now implemented. It still has some (mainly styling) issues (see below) and if there is anything that could still be back ported please tell me
GTK3:
GTK4:
A few general things:
SourceListwas replaced with aListViewand aTreeListModelVirtualizingListBoxwas replaced with aListViewA few issues to note:
Gtk.TreeExpanderhandles indentation. This can be changed with GTK 4.10 however. It's probably best to make GTK 4.10 mandatory anyway because of some bugs it fixes withListView. Also I don't know about the account icons I just added them because I thought they might look good :)Gtk.Panedseparators (at least for me) and the unread badges and their size and alignment (see TODOs). I'm gonna need some help with these :)Prompt for credentials if not found (Turns out it's fixed inE.CredentialsPrompterreplacement ? Or did I miss something?)libedatserverui4versions >=3.46.4(See this issue). Therefore revert changes to that.MessageListItemFeedback and suggestions are always appreciated! 😀