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

[Andrey Smirnov <andrew.smirnov@gmail.com>]

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

On Wed, Dec 14, 2011 at 12:41 PM, Jan Djärv <jan.h.d@swipnet.se> wrote:
> If you Alt-tab in icewm for example, it shows the titles, see
> attached picture.

Now, I see your point. On the other hand in Unity window title doesn't
play that important role in window-switching. Can we compromise on
exposing run-time variable something along the lines of
`gtk-popup-dialogs-show-titles' and then the appearance could be
controlled from user's .emacs?

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

All right, in for a penny in for a pound, how about
`gtk-popup-dialogs-message-format' to control that aspect?

>
> Perhaps, but then we maybe should change all ports?

Do you mean change how dialogs look in all toolkits?
Yes, I think it would be even a better idea, I'll look into it.
Hopefully Lennart would be able to find his patches for w32, and then
they could be used.

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

It still won't change the fact that original
`dialog_selection_callback' duplicates the functionality of
`popup_deactivate_callback'.


I attached reworked version of the patch, it still lacks comments with
description for new functions, but I'll add it as soon as the code
stabilizes.

Andrey Smirnov

[-- Attachment #2: 0001-Change-the-look-of-dialogs-created-with-x-popup-dial.patch --]
[-- Type: text/x-diff, Size: 16346 bytes --]

From 99ccca62b6a2402cba69114b9b86e534511222fd Mon Sep 17 00:00:00 2001
From: Andrey Smirnov <andrew.smirnov@gmail.com>
Date: Sat, 10 Dec 2011 18:13:42 -0800
Subject: [PATCH 1/2] Change the look of dialogs created with
 `x-popup-dialog'(Gtk)

New code creates dialogs that looks(hopefully) more similar to dialogs
created by other Gtk applications such as GEdit. Created dialog no
longer has more than two buttons and multiple choices situation is
handled with radio-buttons.
---
 src/gtkutil.c |  368 +++++++++++++++++++++++++++++++++++++++++++--------------
 src/gtkutil.h |    2 +
 src/xmenu.c   |    2 -
 3 files changed, 279 insertions(+), 93 deletions(-)

diff --git a/src/gtkutil.c b/src/gtkutil.c
index bc71685..7114e4f 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -1398,36 +1398,38 @@ xg_set_frame_icon (FRAME_PTR f, Pixmap icon_pixmap, Pixmap icon_mask)
 /* Return the dialog title to use for a dialog of type KEY.
    This is the encoding used by lwlib.  We use the same for GTK.  */
 
-static const char *
-get_dialog_title (char key)
+static GtkWidget *
+get_dialog_icon (char key)
 {
-  const char *title = "";
+  GtkWidget *image = NULL;
 
   switch (key) {
   case 'E': case 'e':
-    title = "Error";
+    image = gtk_image_new_from_stock (GTK_STOCK_DIALOG_ERROR,
+                                      GTK_ICON_SIZE_DIALOG);
     break;
 
   case 'I': case 'i':
-    title = "Information";
+    image = gtk_image_new_from_stock (GTK_STOCK_DIALOG_INFO,
+                                      GTK_ICON_SIZE_DIALOG);
     break;
 
   case 'L': case 'l':
-    title = "Prompt";
     break;
 
   case 'P': case 'p':
-    title = "Prompt";
     break;
 
   case 'Q': case 'q':
-    title = "Question";
+    image = gtk_image_new_from_stock (GTK_STOCK_DIALOG_QUESTION,
+                                      GTK_ICON_SIZE_DIALOG);
     break;
   }
 
-  return title;
+  return image;
 }
 
+
 /* Callback for dialogs that get WM_DELETE_WINDOW.  We pop down
    the dialog, but return TRUE so the event does not propagate further
    in GTK.  This prevents GTK from destroying the dialog widget automatically
@@ -1446,6 +1448,149 @@ dialog_delete_callback (GtkWidget *w, GdkEvent *event, gpointer user_data)
   return TRUE;
 }
 
+struct xg_popup_dialog_callback_data
+{
+  void (*select_cb) (GtkWidget *, gpointer);
+  void (*deactivate_cb) (GtkWidget *, gpointer);
+  gboolean multichoice_p;
+  GSList *radio_buttons;
+};
+
+static void
+popup_dialog_response_cb (GtkDialog *dialog,
+                          gint response_id,
+                          gpointer user_data)
+{
+  struct xg_popup_dialog_callback_data *data = user_data;
+
+  switch (response_id)
+    {
+    case GTK_RESPONSE_ACCEPT:
+      {
+        GSList *radio_button;
+        for (radio_button = data->radio_buttons;
+             radio_button != NULL;
+             radio_button = radio_button->next)
+          if (gtk_toggle_button_get_active
+              (GTK_TOGGLE_BUTTON (radio_button->data)))
+            {
+              gpointer val_data = g_object_get_data
+                (G_OBJECT (radio_button->data), XG_RADIO_BUTTON_DATA);
+
+              data->select_cb (GTK_WIDGET (dialog), val_data);
+              break;
+            }
+      }
+      break;
+    case GTK_RESPONSE_DELETE_EVENT:
+      data->deactivate_cb (GTK_WIDGET (dialog), NULL);
+      return;
+      break;
+    case GTK_RESPONSE_REJECT:
+      break;
+    default:
+      if (!data->multichoice_p)
+        {
+          GObject *object = G_OBJECT (gtk_dialog_get_widget_for_response
+                                      (GTK_DIALOG (dialog), response_id));
+          gpointer val_data = g_object_get_data (object, XG_RADIO_BUTTON_DATA);
+
+          data->select_cb (GTK_WIDGET (dialog), val_data);
+        }
+      break;
+    }
+
+  data->deactivate_cb (GTK_WIDGET (dialog), NULL);
+  g_free (user_data);
+}
+
+
+
+static inline GtkWidget *
+popup_dialog_create_simple_or_multichoice (gboolean multichoice_p)
+{
+  GtkWidget *dialog;
+  if (multichoice_p)
+    dialog = gtk_dialog_new_with_buttons ("",
+                                          NULL,
+                                          GTK_DIALOG_MODAL |
+                                          GTK_DIALOG_DESTROY_WITH_PARENT,
+                                          GTK_STOCK_OK,
+                                          GTK_RESPONSE_ACCEPT,
+                                          GTK_STOCK_CANCEL,
+                                          GTK_RESPONSE_REJECT,
+                                          NULL);
+  else
+    {
+      dialog = gtk_dialog_new ();
+      g_object_set (dialog,
+                    "destroy-with-parent" , TRUE,
+                    "modal"               , TRUE,
+                    "title"               , "",
+                    NULL);
+    }
+  g_object_set (dialog,
+                "name"                , "emacs-dialog",
+                "resizable"           , FALSE,
+                "skip-taskbar-hint"   , TRUE,
+                NULL);
+
+  return dialog;
+}
+
+static inline void
+popup_dialog_create_and_pack_message (GtkBox *vbox, char *utf8_label)
+{
+  GtkRequisition req;
+  gchar *message, *escaped_text;
+
+  GtkWidget *w = gtk_label_new (NULL);
+  g_object_set (w,
+                "wrap"       , TRUE,
+                "use-markup" , TRUE,
+                "selectable" , TRUE,
+                "xalign"     , 0.0,
+                "yalign"     , 0.5,
+                NULL);
+
+  escaped_text = g_markup_escape_text (utf8_label, -1);
+  message = g_strconcat ("<span weight=\"bold\" size=\"larger\">",
+                         escaped_text, "</span>", NULL);
+
+  gtk_label_set_markup (GTK_LABEL (w), message);
+
+  g_free (message);
+  g_free (escaped_text);
+
+  gtk_box_pack_start (GTK_BOX (vbox), w, TRUE, TRUE, 0);
+
+  /* Try to make dialog look better.  Must realize first so
+     the widget can calculate the size it needs.  */
+  gtk_widget_realize (w);
+  gtk_widget_get_preferred_size (w, NULL, &req);
+  gtk_box_set_spacing (GTK_BOX (vbox), req.height);
+}
+
+static inline GtkWidget *
+popup_dialog_add_radio_button (GtkBox *box,
+                               GtkWidget *previous_radio_button,
+                               char *utf8_label,
+                               gboolean enabled)
+{
+
+  GtkWidget *w = gtk_radio_button_new_with_label_from_widget
+    (GTK_RADIO_BUTTON (previous_radio_button),
+     utf8_label);
+
+  if (!enabled)
+    gtk_widget_set_sensitive (w, FALSE);
+
+  gtk_box_pack_start (GTK_BOX (box), w, TRUE, TRUE, 0);
+
+  return w;
+}
+
+
 /* Create a popup dialog window.  See also xg_create_widget below.
    WV is a widget_value describing the dialog.
    SELECT_CB is the callback to use when a button has been pressed.
@@ -1458,102 +1603,143 @@ create_dialog (widget_value *wv,
                GCallback select_cb,
                GCallback deactivate_cb)
 {
-  const char *title = get_dialog_title (wv->name[0]);
-  int total_buttons = wv->name[1] - '0';
-  int right_buttons = wv->name[4] - '0';
-  int left_buttons;
-  int button_nr = 0;
-  int button_spacing = 10;
-  GtkWidget *wdialog = gtk_dialog_new ();
-  GtkDialog *wd = GTK_DIALOG (wdialog);
-  GtkBox *cur_box = GTK_BOX (gtk_dialog_get_action_area (wd));
-  widget_value *item;
-  GtkWidget *whbox_down;
+  int total_buttons      = wv->name[1] - '0';
+  gboolean multichoice_p = total_buttons > 2;
+
+  /*
+     Depending on the `total_buttons' value the dialog created can be
+    of two types:
+
+    1. when multichoice_p is TRUE, then resulting dialog has following
+    structure:
+
+    +------- content_area -------------------------+
+    | +-------- hbox ----------------------------+ |
+    | | +-------+ +-- vbox --------------------+ | |
+    | | |       | | <message>                  | | |
+    | | | image | | +------ choices_box -----+ | | |
+    | | |       | | |  o first item          | | | |
+    | | |       | | |  o second item         | | | |
+    | | |       | | |  o third item          | | | |
+    | | |       | | |  ...                   | | | |
+    | | |       | | +------------------------+ | | |
+    | | +-------+ +----------------------------+ | |
+    | +------------------------------------------+ |
+    +----------------------------------------------+
+    +------- action_area --------------------------+
+    |                     +------+  +--------+     |
+    |                     |  OK  |  | Cancel |     |
+    |                     +------+  +--------+     |
+    +----------------------------------------------+
+
+
+    2. when multichoice_p is FALSE, its structure is this:
+
+    +------------- content_area ------------------------------+
+    |  +--------- hbox -------------------------------------+ |
+    |  | +---------+ +------ vbox ------------------------+ | |
+    |  | |         | | <message>                          | | |
+    |  | | image   | |                                    | | |
+    |  | |         | |                                    | | |
+    |  | +---------+ +------------------------------------+ | |
+    |  +----------------------------------------------------+ |
+    +---------------------------------------------------------+
+    +------- action_area -------------------------------------+
+    |                             +---------+  +--------+     |
+    |                             |  Item 1 |  | Item 2 |     |
+    |                             +---------+  +--------+     |
+    +---------------------------------------------------------+
+
+   */
+
+
+  GtkWidget *dialog = popup_dialog_create_simple_or_multichoice (multichoice_p);
+  GtkWidget *content_area, *hbox, *vbox, *frame, *choices_box;
+  GtkWidget *w = NULL;
+
+
+  content_area = gtk_dialog_get_content_area (GTK_DIALOG (dialog));
+
+  hbox = gtk_hbox_new (FALSE, 12);
+  gtk_container_set_border_width (GTK_CONTAINER (hbox), 5);
+
+  vbox = gtk_vbox_new (FALSE, 12);
 
-  /* If the number of buttons is greater than 4, make two rows of buttons
-     instead.  This looks better.  */
-  int make_two_rows = total_buttons > 4;
-
-  if (right_buttons == 0) right_buttons = total_buttons/2;
-  left_buttons = total_buttons - right_buttons;
+  {
+    GtkWidget *image = get_dialog_icon (wv->name[0]);
+    gtk_misc_set_alignment (GTK_MISC (image), 0.5, 0.0);
 
-  gtk_window_set_title (GTK_WINDOW (wdialog), title);
-  gtk_widget_set_name (wdialog, "emacs-dialog");
+    gtk_box_pack_start (GTK_BOX (hbox), image, FALSE, FALSE, 0);
+  }
 
+  gtk_box_pack_start (GTK_BOX (hbox), vbox, FALSE, FALSE, 0);
+  gtk_container_add (GTK_CONTAINER (content_area), hbox);
 
-  if (make_two_rows)
+  if (multichoice_p)
     {
-      GtkWidget *wvbox = gtk_vbox_new (TRUE, button_spacing);
-      GtkWidget *whbox_up = gtk_hbox_new (FALSE, 0);
-      whbox_down = gtk_hbox_new (FALSE, 0);
+      frame  = gtk_frame_new ("Possible actions/answers");
+      choices_box = gtk_vbox_new (FALSE, 1);
+    }
 
-      gtk_box_pack_start (cur_box, wvbox, FALSE, FALSE, 0);
-      gtk_box_pack_start (GTK_BOX (wvbox), whbox_up, FALSE, FALSE, 0);
-      gtk_box_pack_start (GTK_BOX (wvbox), whbox_down, FALSE, FALSE, 0);
+  {
+    widget_value *item;
+    gint button_number = 0;
+    for (item = wv->contents; item; item = item->next)
+      {
+        char *utf8_label = get_utf8_string (item->value);
 
-      cur_box = GTK_BOX (whbox_up);
+        if (g_strcmp0 (item->name, "message") == 0)
+          popup_dialog_create_and_pack_message (GTK_BOX (vbox), utf8_label);
+        else
+          {
+            if (!multichoice_p)
+              {
+                gtk_dialog_add_button (GTK_DIALOG (dialog),
+                                       utf8_label, button_number);
+                w = gtk_dialog_get_widget_for_response (GTK_DIALOG (dialog),
+                                                        button_number++);
+              }
+            else
+              w = popup_dialog_add_radio_button (GTK_BOX (choices_box), w,
+                                                 utf8_label,
+                                                 item->enabled);
+            g_object_set_data (G_OBJECT (w), XG_RADIO_BUTTON_DATA,
+                               item->call_data);
+          }
+
+        g_free (utf8_label);
+      }
+  }
+
+  if (multichoice_p)
+    {
+      gtk_container_add (GTK_CONTAINER (frame), choices_box);
+      gtk_box_pack_start (GTK_BOX (vbox), frame, TRUE, TRUE, 0);
     }
 
-  g_signal_connect (G_OBJECT (wdialog), "delete-event",
+  g_signal_connect (G_OBJECT (dialog), "delete-event",
                     G_CALLBACK (dialog_delete_callback), 0);
 
   if (deactivate_cb)
-    {
-      g_signal_connect (G_OBJECT (wdialog), "close", deactivate_cb, 0);
-      g_signal_connect (G_OBJECT (wdialog), "response", deactivate_cb, 0);
-    }
+      g_signal_connect (G_OBJECT (dialog), "close", deactivate_cb, 0);
 
-  for (item = wv->contents; item; item = item->next)
+  if (select_cb)
     {
-      char *utf8_label = get_utf8_string (item->value);
-      GtkWidget *w;
-      GtkRequisition req;
+      struct xg_popup_dialog_callback_data *data = g_malloc (sizeof (*data));
 
-      if (item->name && strcmp (item->name, "message") == 0)
-        {
-          GtkBox *wvbox = GTK_BOX (gtk_dialog_get_content_area (wd));
-          /* This is the text part of the dialog.  */
-          w = gtk_label_new (utf8_label);
-          gtk_box_pack_start (wvbox, gtk_label_new (""), FALSE, FALSE, 0);
-          gtk_box_pack_start (wvbox, w, TRUE, TRUE, 0);
-          gtk_misc_set_alignment (GTK_MISC (w), 0.1, 0.5);
-
-          /* Try to make dialog look better.  Must realize first so
-             the widget can calculate the size it needs.  */
-          gtk_widget_realize (w);
-          gtk_widget_get_preferred_size (w, NULL, &req);
-          gtk_box_set_spacing (wvbox, req.height);
-	  if (item->value && strlen (item->value) > 0)
-            button_spacing = 2*req.width/strlen (item->value);
-        }
-      else
-        {
-          /* This is one button to add to the dialog.  */
-          w = gtk_button_new_with_label (utf8_label);
-          if (! item->enabled)
-            gtk_widget_set_sensitive (w, FALSE);
-          if (select_cb)
-            g_signal_connect (G_OBJECT (w), "clicked",
-                              select_cb, item->call_data);
-
-          gtk_box_pack_start (cur_box, w, TRUE, TRUE, button_spacing);
-          if (++button_nr == left_buttons)
-            {
-              if (make_two_rows)
-                cur_box = GTK_BOX (whbox_down);
-              else
-                gtk_box_pack_start (cur_box,
-                                    gtk_label_new (""),
-                                    TRUE, TRUE,
-                                    button_spacing);
-            }
-        }
+      data->select_cb = (void (*) (GtkWidget *, gpointer)) select_cb;
+      data->multichoice_p = multichoice_p;
+      data->radio_buttons = (multichoice_p) ?
+        gtk_radio_button_get_group (GTK_RADIO_BUTTON (w)) : NULL;
+
+      if (deactivate_cb)
+        data->deactivate_cb = (void (*) (GtkWidget *, gpointer)) deactivate_cb;
 
-     if (utf8_label)
-       g_free (utf8_label);
+      g_signal_connect (G_OBJECT (dialog), "response",
+                        G_CALLBACK (popup_dialog_response_cb), data);
     }
 
-  return wdialog;
+  return dialog;
 }
 
 struct xg_dialog_data
diff --git a/src/gtkutil.h b/src/gtkutil.h
index 7cc2d21..9c55fa6 100644
--- a/src/gtkutil.h
+++ b/src/gtkutil.h
@@ -38,6 +38,8 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 /* Key for data that menu items hold.  */
 #define XG_ITEM_DATA "emacs_menuitem"
 
+#define XG_RADIO_BUTTON_DATA "emacs_radio_button"
+
 /* This is a list node in a generic list implementation.  */
 typedef struct xg_list_node_
 {
diff --git a/src/xmenu.c b/src/xmenu.c
index 4b7bbfd..547b538 100644
--- a/src/xmenu.c
+++ b/src/xmenu.c
@@ -1904,8 +1904,6 @@ dialog_selection_callback (GtkWidget *widget, gpointer client_data)
      as long as pointers have enough bits to hold small integers.  */
   if ((intptr_t) client_data != -1)
     menu_item_selection = (Lisp_Object *) client_data;
-
-  popup_activated_flag = 0;
 }
 
 /* Pop up the dialog for frame F defined by FIRST_WV and loop until the
-- 
1.7.5.4