Conversation
Marukesu
left a comment
There was a problem hiding this comment.
I like the idea of having an Application class, but i think the boilerplate for elementary applications is minimal enough to not worth the addition. if we have more features than theme and the unity badge support (that not many elementary apps make use that i known of) ready to be merged, then it's start to being considerable. Note that, granite used to have an Application class, but got deprecated and replaced with the current Granite.Service.Application namespace.
We also would want to make most applications class make use of what Gtk.Application already provide us with before setling in what we need to provide. Like, the quit action implemented here would break code, terminal, and files; as they support multiple windows and need to manage they state before quit. But we also need to check if we can move that management to the shutdown signal. If so, then the quit action that is currently implemented here works.
There was a problem hiding this comment.
This and Granite.Services.Application should be merged, also Application classes aren't widgets, so this file should be direct at the src folder.
| @@ -0,0 +1,53 @@ | |||
| /* | |||
| * Copyright 2025 elementary, Inc. (https://elementary.io) | |||
| * SPDX-License-Identifier: GPL-2.0-or-later | |||
There was a problem hiding this comment.
Pretty sure we use LGPL-3.0-or-later in granite.
| public Application () { | ||
| Object ( | ||
| flags: ApplicationFlags.DEFAULT_FLAGS | ||
| ); | ||
| } |
There was a problem hiding this comment.
Public constructors, in this case, is only for the use of procedural code (i.e., c and rust bindings). So this should take the application_id and flags as parameters.
| public Application () { | |
| Object ( | |
| flags: ApplicationFlags.DEFAULT_FLAGS | |
| ); | |
| } | |
| public Application (string? application_id, GLib.ApplicationFlags flags) { | |
| Object ( | |
| application_id: application_id, | |
| flags: flags | |
| ); | |
| } |
Otherwise, it has no use for us in the vala side of the things.
| Intl.setlocale (LocaleCategory.ALL, ""); | ||
| Intl.bindtextdomain (GETTEXT_PACKAGE, LOCALEDIR); | ||
| Intl.bind_textdomain_codeset (GETTEXT_PACKAGE, "UTF-8"); | ||
| Intl.textdomain (GETTEXT_PACKAGE); |
There was a problem hiding this comment.
Granite.init() should be handling this already, also, we don't have any granite specific command line argument to this be executed before startup() anyway.
There was a problem hiding this comment.
Yeah it looks like init does handle some of this already:
Line 30 in 66c5fec
| gtk_settings.gtk_application_prefer_dark_theme = ( | ||
| granite_settings.prefers_color_scheme == DARK | ||
| ); | ||
|
|
||
| granite_settings.noftify["prefers-color-scheme"].connect (() => { | ||
| gtk_settings.gtk_application_prefer_dark_theme = ( | ||
| granite_settings.prefers_color_scheme == DARK | ||
| ); | ||
| }); |
There was a problem hiding this comment.
If we are going to handling this here, we need a property that controls the "no-preference" behavior between default dark and default bright applications.
There was a problem hiding this comment.
We actually already handle this in StyleManager as well. I think that's automatically added in Granite.init ()
There was a problem hiding this comment.
Ah, right, the flatpak SDK doesn't have the 7.7.0 stuff, so my boilerplate reference code is old* 😓
| var quit_action = new SimpleAction ("quit", null); | ||
| add_action (quit_action); | ||
| set_accels_for_action ("app.quit", {"<Control>q"}); | ||
| quit_action.activate.connect (quit); |
There was a problem hiding this comment.
Application actions can be activated from external applications after added, so we need to have it completely setup before adding it.
| var quit_action = new SimpleAction ("quit", null); | |
| add_action (quit_action); | |
| set_accels_for_action ("app.quit", {"<Control>q"}); | |
| quit_action.activate.connect (quit); | |
| var quit_action = new SimpleAction ("quit", null); | |
| quit_action.activate.connect (quit); | |
| add_action (quit_action); | |
| set_accels_for_action ("app.quit", {"<Control>q"}); |
| var granite_settings = Granite.Settings.get_default (); | ||
| var gtk_settings = Gtk.SEttings.get_default (); |
There was a problem hiding this comment.
Little typo, also thoses objects are unowned.
| var granite_settings = Granite.Settings.get_default (); | |
| var gtk_settings = Gtk.SEttings.get_default (); | |
| unowned var granite_settings = Granite.Settings.get_default (); | |
| unowned var gtk_settings = Gtk.Settings.get_default (); |
Moves some common boilerplate code to a
Gtk.Applicationsubclass so developers get it for free.