GNOME Bugzilla – Bug 711308
Some fixes for the save confirmation dialog
Last modified: 2018-03-26 15:32:44 UTC
Here are a few patches for the save confirmation dialog.
Created attachment 258819 [details] [review] Fix display of save error details Make sure we show enough height in the scrolled window and correctly set the scroll policy.
Created attachment 258820 [details] [review] Use Cancel/Save buttons in save confirmation dialog Instead of Yes/No as recommended by the HIG.
Created attachment 258821 [details] [review] Align details window with action buttons
Created attachment 258822 [details] [review] Use proper quotes in save confirmation message
Created attachment 258823 [details] [review] Don't use markup in the message dialog labels This allows the dialog to use the default bold style.
Created attachment 258824 [details] screenshot (before)
Thanks Jon, I will take a look at this ASAP BTW does this qualify as a UI break?
Review of attachment 258819 [details] [review]: This patch causes the error window in the save confirmation dialog to not scroll horizontally and wrap instead. That is a problem, this error message should definitely not wrap in the way it's currently formatted. Each line is a warning message pertaining to a specific widget, property or signal, and each line is prefixed with the full path to that element on the same line. The path is generated with glade_widget_generate_path_name() and for a deep widget it can be pretty long... wrapping text in there will just cause mayhem. I'm totally open to a change that actually improves this though, one thought is that since warnings now show up with icons and warnings in the inspector, we could show the inspector treeview in the scrolled window with the rows filtered down to only objects which have errors (and their parents of course, so one can still see the tree).
Review of attachment 258820 [details] [review]: Looks good. Please make the same change you did for gladeui/glade-project.c also in src/glade-window.c In do_save(), where it asks "Failed to backup existing file, continue saving ?". And then commit.
Review of attachment 258821 [details] [review]: Correct, the child of the dialogs content "vbox" must have a border width of 5, ugly but true ;-)
Review of attachment 258822 [details] [review]: Why the fancy quote characters ? Is it wise to apply this to projects directly as a hard coded utf-8 character ? Not better to push for a set of well defined macros in some gnome common package first ? GNOME_ELLIPSES, GNOME_CUTE_QUOTE_START, GNOME_CUTE_QUOTE_END ? By the looks of it, you already missed over 90% of the instances where quotes are used and displayed in warnings and dialogs... that must be an indication that we're going about enforcing UI consistency in the wrong way. I.e. consult: grep -r -i "\\\\\\\"" `find . -name "*.c"` (yeah looks weird... through bash & grep you need a lot of back slashes to match: \")
Review of attachment 258823 [details] [review]: Since GTK+ API is completely awkward in this regard, I'd rather you add a comment around that. Calling gtk_message_dialog_set_markup() followed by g_object_set (dialog, "use-markup", FALSE, NULL); screams at me... saying: "Tristan you silly bastard, why arent you just calling gtk_message_dialog_set_text() ?" Would be great to have a comment pointing an accusing finger at GtkMessageDialog API along with that... then please commit ;-)
(In reply to comment #9) > Review of attachment 258820 [details] [review]: > > Looks good. > > Please make the same change you did for gladeui/glade-project.c also in > src/glade-window.c > > In do_save(), where it asks "Failed to backup existing file, continue saving > ?". > > And then commit. For the GLADE_UI_ARE_YOU_SURE dialog? Those seem to have OK/Cancel and not Yes/No I think.
Currently, I do a copy / paste of the error message to emacs and format it via macros, to have a good look of the warnings. Thanks !
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glade/issues/142.