From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Andrey Smirnov Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Change the look of dialogs created with `x-popup-dialog' Date: Tue, 13 Dec 2011 21:29:58 -0800 Message-ID: References: <890BD378-D3CE-41CE-AEC0-7CEAA1D8CE05@swipnet.se> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-Trace: dough.gmane.org 1323840610 2645 80.91.229.12 (14 Dec 2011 05:30:10 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Wed, 14 Dec 2011 05:30:10 +0000 (UTC) Cc: emacs-devel@gnu.org To: =?ISO-8859-1?Q?Jan_Dj=E4rv?= Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Dec 14 06:30:06 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([140.186.70.17]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1RahPp-0005T7-Ie for ged-emacs-devel@m.gmane.org; Wed, 14 Dec 2011 06:30:05 +0100 Original-Received: from localhost ([::1]:46968 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RahPp-0007mm-2f for ged-emacs-devel@m.gmane.org; Wed, 14 Dec 2011 00:30:05 -0500 Original-Received: from eggs.gnu.org ([140.186.70.92]:41220) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RahPl-0007lD-Vj for emacs-devel@gnu.org; Wed, 14 Dec 2011 00:30:03 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RahPk-0000an-7t for emacs-devel@gnu.org; Wed, 14 Dec 2011 00:30:01 -0500 Original-Received: from mail-iy0-f169.google.com ([209.85.210.169]:49530) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RahPk-0000aR-4j for emacs-devel@gnu.org; Wed, 14 Dec 2011 00:30:00 -0500 Original-Received: by iahk25 with SMTP id k25so802854iah.0 for ; Tue, 13 Dec 2011 21:29:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=qStLumvbxABXqcw+uhLVKric7DxB5YvMsw1a+ja1vmM=; b=WVETZRwLYleZwq2e7w1tnJQNK1p750kvXon9RqW+4PcYm9PyR0y25u9YwP52ye18LB KccL5StXS8fSIwp15pI3Vu1FoN2r3PrmPbfj7nQosE2wAARbUwOGXAIiHpj+F5XGkup6 AE9IFDiihNYOR0mMO4VVC5yb1QAqGPsJSr6uA= Original-Received: by 10.50.220.164 with SMTP id px4mr22749116igc.8.1323840598719; Tue, 13 Dec 2011 21:29:58 -0800 (PST) Original-Received: by 10.50.237.74 with HTTP; Tue, 13 Dec 2011 21:29:58 -0800 (PST) In-Reply-To: <890BD378-D3CE-41CE-AEC0-7CEAA1D8CE05@swipnet.se> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 209.85.210.169 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:146702 Archived-At: On Mon, Dec 12, 2011 at 11:07 PM, Jan Dj=E4rv wrote: > Hello. > > The change looks good in principle, the current dialog when closing > Emacs with unsaved changes contains way too many buttons. =A0How 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? > Should the radiobuttons be grouped into two columns when the exceed > some number, say 4 or 6 for example? I don't know, since the upper limit on the amount of buttons is 9, and 9 radio-buttons in the same row looks fine to my taste, in my opinion they shouldn't be, but then again that's the matter of personal taste. > > The code has issues however: > > You have removed the title, that is no good. =A0Several 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. > > The use of default bold indicates screaming to me. I thought that function was reserved for all caps :) > 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? > > 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. Didn't think of that one. Thanks, will fix that. > > 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. > > > + =A0 =A0 =A0 =A0 =A0g_signal_connect (G_OBJECT (ok), "clicked", deactiva= te_cb, 0); > + =A0 =A0 =A0 =A0 =A0g_signal_connect (G_OBJECT (cancel), "clicked", sele= ct_cb, 0); > + =A0 =A0 =A0 =A0 =A0g_signal_connect (G_OBJECT (cancel), "clicked", deac= tivate_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. > > - =A0popup_activated_flag =3D 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. At least for Gtk this code is redundant anyway, since the same functionality could, and IMHO should, be achieved by chaining `select_cb' and `deactivate_cb' for particular signal. Not sure if all the other toolkits allow you to do such a thing. Andrey Smirnov