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

["Jan Djärv" <jan.h.d@swipnet.se>]

[-- Attachment #1: Type: text/plain, Size: 3297 bytes --]

Hello.

14 dec 2011 kl. 06:29 skrev Andrey Smirnov:

> On Mon, Dec 12, 2011 at 11:07 PM, Jan Djärv <jan.h.d@swipnet.se> wrote:
>> 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?
> 
> I, attached before/after screenshot to my initial e-mail, did you see
> it? Or do you want to see the results of different code sample?

I checked, 6 looks quite Ok.

>> 
>> 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.
> 
> Well, the title is pretty much useless, since it doesn't provide any
> information, and I did set the "skip-taskbar-hint" to TRUE, so it
> shouldn't appear in the task bar. What icon bars are we talking about?
> I used GEdit as some sort of reference implementation of how dialogs of
> GNOME editors should look like, and it's dialogs don't have the title.
> I don't mind returning it back, it would be  a trivial change after
> all, it's just I'm not sure that I see the point.

If you Alt-tab in icewm for example, it shows the titles, see attached picture.


> 
>> 
>> The use of default bold indicates screaming to me.
> 
> I thought that function was reserved for all caps :)
> 

A lot of shouting going on...

>> It should not be default. A configure option is OK. Ditto font
>> size larger. Not sure if this is a good default.
> 
> Well, again I tried to emulate GEdit, whose dialogs I find to be
> easier on the eyes. And don't you think it would be too minor detail
> to be worth a dedicated configure option. Shouldn't we just some sort
> of a consensus and just use that settings?

Perhaps, but then we maybe should change all ports?

> 
> 
>> 
>> 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.
> 
> Well, I guess there is more than one way to skin a cat, I'll rewrite
> that portion of the code.
> 
>> 
>> 
>> +          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?
> 
> Because I need to set `menu_item_selection' to 0 otherwise if user
> fiddles with radiobuttons and then presses cancel button for the code
> in xmenu.c it would be indistinguishable from OK button being pressed.
> 
>> 
>> -  popup_activated_flag = 0;
>> 
>> Do not remove this line. It is needed. Why are you removing it?
> 
> It is removed because otherwise one wouldn't be able to change
> selected radio button more than once, since the first call to
> `select_cb' would close the whole dialog.

These two issues will go away if you defer calling callbacks until OK is pressed.

	Jan D.



[-- Attachment #2.1: Type: text/html, Size: 5465 bytes --]

[-- Attachment #2.2: dialog.png --]
[-- Type: image/png, Size: 66903 bytes --]