@b4n commented on this pull request.
Changes look pretty good, see inline comments.
Seems to work nicely as well. I didn't test it entirely thoroughly, but basic
open/save/select folder seem to work find with or without the option on Linux.
> @@ -1311,6 +1311,22 @@
<property name="position">3</property>
</packing>
</child>
+ <child>
+ <object class="GtkCheckButton"
id="check_native_dialogs">
+ <property name="label"
translatable="yes">Use platform-native file dialogs</property>
+ <property name="visible">True</property>
+ <property name="can-focus">True</property>
+ <property
name="receives-default">False</property>
+ <property name="tooltip-text"
translatable="yes">Defines whether to use the platform-native file dialogs or
whether to use the GTK default dialogs</property>
We need asking @elextr, but wouldn't it be better English worded without the
second "whether"? *Defines whether to use the platform-native file dialogs or
the GTK default dialogs*
> @@ -1311,6 +1311,22 @@
<property name="position">3</property>
</packing>
</child>
+ <child>
+ <object class="GtkCheckButton"
id="check_native_dialogs">
+ <property name="label"
translatable="yes">Use platform-native file dialogs</property>
Could you please add a mnemonic? It's sometimes tricky to find one that's not
used yet (here in LANGUAGE=C), but it's really nice to have them :)
I see other items around also lack it, but we could start now :) (or I'll try
and do that after this landed)
> + if (GTK_IS_WIDGET(dialog))
+ return gtk_dialog_run(GTK_DIALOG(dialog));
+
+ return gtk_native_dialog_run(GTK_NATIVE_DIALOG(dialog));
Wouldn't it make more sense to do something like:
```suggestion
if (GTK_IS_NATIVE_DIALOG(dialog))
return gtk_native_dialog_run(GTK_NATIVE_DIALOG(dialog));
else
return gtk_dialog_run(GTK_DIALOG(dialog));
```
Or at least `GTK_IS_DIALOG()`, as you'd then could reasonably expect
`gtk_dialog_run()` to work on it. My point is mostly that this relies on the
fact `GtkNativeDialog` isn't a `GtkWidget`, and although it seems to make sense
I wouldn't rule out a fallback implementation to be one at some point, but not
necessarily be a `GtkDialog` (could be a window with a custom
`gtk_native_dialog_run()` implementation).
Same for the other occurrence of this.
> - gtk_dialog_set_default_response(GTK_DIALOG(dialog),
> GTK_RESPONSE_ACCEPT);
-
- gtk_widget_set_size_request(dialog, -1, 460);
- gtk_window_set_modal(GTK_WINDOW(dialog), TRUE);
- gtk_window_set_destroy_with_parent(GTK_WINDOW(dialog), TRUE);
- gtk_window_set_skip_taskbar_hint(GTK_WINDOW(dialog), FALSE);
- gtk_window_set_type_hint(GTK_WINDOW(dialog),
GDK_WINDOW_TYPE_HINT_DIALOG);
- gtk_window_set_transient_for(GTK_WINDOW(dialog),
GTK_WINDOW(main_widgets.window));
- gtk_file_chooser_set_select_multiple(GTK_FILE_CHOOSER(dialog), TRUE);
- gtk_file_chooser_set_local_only(GTK_FILE_CHOOSER(dialog), FALSE);
-
- /* add checkboxes and filename entry */
- gtk_file_chooser_set_extra_widget(GTK_FILE_CHOOSER(dialog),
add_file_open_extra_widget(dialog));
+ if (interface_prefs.use_native_windows_dialogs)
+ dialog = GTK_FILE_CHOOSER(gtk_file_chooser_native_new(_("Open
File"),
+ GTK_WINDOW(main_widgets.window),
GTK_FILE_CHOOSER_ACTION_OPEN, _("_Open"), _("_Cancel")));
Any reason not to use the default labels here? They seem to be Open/Cancel
just the same, doesn't them? Or is it problematic on macos/Windows? At least
it works great on Linux, and avoids having additional strings here. Moreover,
I would think leaving them default would help with platform consistency (which
is the goal here, isn't it?) -- unless again if it's really problematic, but
I'd be surprised it'd be given the basic nature of our use cases.
Same applies to all other uses.
```suggestion
GTK_WINDOW(main_widgets.window),
GTK_FILE_CHOOSER_ACTION_OPEN, NULL, NULL));
```
> @@ -461,18 +490,19 @@ void dialogs_show_open_file(void)
SETPTR(initdir, utils_get_locale_from_utf8(initdir));
dialog = create_open_file_dialog();
- open_file_dialog_apply_settings(dialog);
+ if (GTK_IS_WIDGET(dialog))
maybe here too
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3861#pullrequestreview-2105848154
You are receiving this because you are subscribed to this thread.
Message ID: <geany/geany/pull/3861/review/[email protected]>
