Hello. 14 dec 2011 kl. 06:29 skrev Andrey Smirnov: > On Mon, Dec 12, 2011 at 11:07 PM, Jan Djärv 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.