From eb1e3f29df90475d37d031a130440f074a358617 Mon Sep 17 00:00:00 2001
From: Maciej Bartczak <39600846+maciekbartczak@users.noreply.github.com>
Date: Fri, 14 Feb 2025 21:16:46 +0100
Subject: [PATCH] code review: - propagate the banner dismissal to all windows
- the dismissed flag is now a local field - avoid repetition in format string
---
src/apprt/gtk/App.zig | 5 +++--
src/apprt/gtk/Window.zig | 36 +++++++++++++++++++++++++-----------
2 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/src/apprt/gtk/App.zig b/src/apprt/gtk/App.zig
index 3e64a52b7..859568fab 100644
--- a/src/apprt/gtk/App.zig
+++ b/src/apprt/gtk/App.zig
@@ -44,8 +44,6 @@ const log = std.log.scoped(.gtk);
pub const Options = struct {};
-/// A global variable indicating if user has dismissed the notice about existing crash reports
-pub var crash_reports_notice_dismissed = false;
core_app: *CoreApp,
config: Config,
@@ -56,6 +54,9 @@ ctx: *c.GMainContext,
/// True if the app was launched with single instance mode.
single_instance: bool,
+/// A flag indicating if user has dismissed the notice about existing crash reports
+crash_reports_notice_dismissed: bool = false,
+
/// The "none" cursor. We use one that is shared across the entire app.
cursor_none: ?*c.GdkCursor,
diff --git a/src/apprt/gtk/Window.zig b/src/apprt/gtk/Window.zig
index a282ca2d3..8f05e9b0f 100644
--- a/src/apprt/gtk/Window.zig
+++ b/src/apprt/gtk/Window.zig
@@ -59,6 +59,8 @@ adw_tab_overview_focus_timer: ?c.guint = null,
wayland: ?wayland.SurfaceState,
+crash_notice_widget: ?*c.GtkWidget = null,
+
pub fn create(alloc: Allocator, app: *App) !*Window {
// Allocate a fixed pointer for our window. We try to minimize
// allocations but windows and other GUI requirements are so minimal
@@ -580,7 +582,7 @@ pub fn sendToast(self: *Window, title: [:0]const u8) void {
}
fn displayCrashReportsNoticeIfNeeded(self: *Window, app: *App, parent_widget: *c.GtkWidget) !void {
- if (App.crash_reports_notice_dismissed) {
+ if (app.crash_reports_notice_dismissed) {
return;
}
@@ -591,9 +593,9 @@ fn displayCrashReportsNoticeIfNeeded(self: *Window, app: *App, parent_widget: *c
if (try it.next()) |_| {
const warning_text = try std.fmt.allocPrintZ(self.app.core_app.alloc,
- \\ ⚠️ There are existing crash reports located at {s}.
+ \\ ⚠️ There are existing crash reports located at {0s}.
\\ If possible, report them to the developers to help improve the application.
- , .{ crash_dir.path, crash_dir.path });
+ , .{crash_dir.path});
defer self.app.core_app.alloc.free(warning_text);
const warning_box = c.gtk_box_new(c.GTK_ORIENTATION_VERTICAL, 0);
@@ -602,6 +604,7 @@ fn displayCrashReportsNoticeIfNeeded(self: *Window, app: *App, parent_widget: *c
adwaita.versionAtLeast(1, 3, 0))
{
const banner = c.adw_banner_new(warning_text.ptr);
+ self.crash_notice_widget = banner;
c.adw_banner_set_revealed(@ptrCast(banner), 1);
c.adw_banner_set_use_markup(@ptrCast(banner), 1);
@@ -610,13 +613,14 @@ fn displayCrashReportsNoticeIfNeeded(self: *Window, app: *App, parent_widget: *c
banner,
"button-clicked",
c.G_CALLBACK(&adwHideBanner),
- banner,
+ self,
null,
c.G_CONNECT_DEFAULT,
);
c.gtk_box_append(@ptrCast(warning_box), @ptrCast(banner));
} else {
+ self.crash_notice_widget = warning_box;
const center_box = c.gtk_center_box_new();
const message = c.gtk_label_new(null);
@@ -631,7 +635,7 @@ fn displayCrashReportsNoticeIfNeeded(self: *Window, app: *App, parent_widget: *c
c.gtk_widget_set_margin_top(@ptrCast(close_button), 10);
c.gtk_widget_set_margin_bottom(@ptrCast(close_button), 10);
c.gtk_widget_set_margin_end(@ptrCast(close_button), 10);
- _ = c.g_signal_connect_data(close_button, "clicked", c.G_CALLBACK(>kHideWidget), warning_box, null, 0);
+ _ = c.g_signal_connect_data(close_button, "clicked", c.G_CALLBACK(>kHideWidget), self, null, 0);
c.gtk_center_box_set_center_widget(@ptrCast(center_box), message);
c.gtk_center_box_set_end_widget(@ptrCast(center_box), close_button);
@@ -1032,15 +1036,25 @@ fn gtkActionReset(
}
fn gtkHideWidget(_: ?*c.GtkWidget, data: ?*anyopaque) void {
- const widget: *c.GtkWidget = @ptrCast(@alignCast(data));
- c.gtk_widget_set_visible(widget, c.FALSE);
- App.crash_reports_notice_dismissed = true;
+ const self = userdataSelf(data.?);
+ for (self.app.core_app.surfaces.items) |surface| {
+ const gtk_surface: *Surface = @ptrCast(surface);
+ if (gtk_surface.container.window()) |window| {
+ c.gtk_widget_set_visible(window.crash_notice_widget, c.FALSE);
+ }
+ }
+ self.app.crash_reports_notice_dismissed = true;
}
fn adwHideBanner(_: ?*c.GtkWidget, data: ?*anyopaque) void {
- const banner: *c.AdwBanner = @ptrCast(data);
- c.adw_banner_set_revealed(banner, c.FALSE);
- App.crash_reports_notice_dismissed = true;
+ const self = userdataSelf(data.?);
+ for (self.app.core_app.surfaces.items) |surface| {
+ const gtk_surface: *Surface = @ptrCast(surface);
+ if (gtk_surface.container.window()) |window| {
+ c.adw_banner_set_revealed(@ptrCast(window.crash_notice_widget), c.FALSE);
+ }
+ }
+ self.app.crash_reports_notice_dismissed = true;
}
/// Returns the surface to use for an action.