Re: [PATCH] Change the look of dialogs created with `x-popup-dialog'

[Jan Djärv]

Hello.

The change looks good in principle, the current dialog when closing Emacs with 
unsaved changes contains way too many buttons.  How does that look on your new 
implementation?  Should the radiobuttons be grouped into two columns when the 
exceed some number, say 4 or 6 for example?

The code has issues however:

You have removed the title, that is no good.  Several window managers show the 
title in icon bars and other places, so keep the title even if you add an icon.

The use of default bold indicates screaming to me.  It should not be default.  
A configure option is OK.
Ditto font size larger.  Not sure if this is a good default.

The use of markup opens up the possibility of having markup in the text for the 
dialog.  Also, some characters will not display right, as they will be 
interpretered as masrkup.  You need to escape the text with g_markup_escape in 
glib.

Radio buttons should not have any callbacks.  When OK is pressed you should 
check what radiobutton is selected and then call the callback.  Just selecting 
a radio button should not execute any code if there are OK and Cancel buttons 
present.  Also the select callback pops down the dialog.  This is not something 
a radiobutton should do.


+          g_signal_connect (G_OBJECT (ok), "clicked", deactivate_cb, 0);
+          g_signal_connect (G_OBJECT (cancel), "clicked", select_cb, 0);
+          g_signal_connect (G_OBJECT (cancel), "clicked", deactivate_cb, 0);

Why select_cb on the cancel button?  It is not right.  You should make a new 
function that ok is bound to that extracts the selected radiobutton and calls 
select_cb.


-  popup_activated_flag = 0;

Do not remove this line.  It is needed.  Why are you removing it?

        Jan D.

11 dec 2011 kl. 16:01 skrev Andrey Smirnov:

> Hello everybody,
> 
> I often close emacs with mouse by pressing 'close' buttons of its
> window and therefore I often encounter questions presented in form of
> dialogs produced by `x-popup-dialog' function. I know it is the matter
> of one's taste, but I think that appearance of aforementioned dialogs
> could be improved by making them look more similar to those created by
> other Gtk and GNOME applications(I used GEdit as a template). The
> patch in attachment is my attempt to implements such changes. To
> summarize it does the following:
> 
>  - Adds, depending on the dialog type, appropriate icon to the dialog,
>    one of GTK_STOCK_DIALOG_INFO or GTK_STOCK_DIALOG_QUESTION.
>  - Removes resize button from the resulting dialog window.
>  - For dialogs with more than two buttons, radio-buttons instead of
>    regular buttons are created.
>  - Makes the question to be rendered in bold font.
> 
> The difference between the results of `x-popup-dialog' can be seen on
> the image in the attachment.
> 
> I guess, there are probably many improvements that can be added both
> to the dialog's appearance and to other code of the patch, so at this
> point I would like you to ask to comment on the patch.
> 
> Andrey Smirnov
> <0001-Change-the-look-of-dialogs-created-with-x-popup-dial.patch><before-after.png>