Re: dialog iteration

[Tim Janik <timj gtk org>]

On 26 Mar 2000, Havoc Pennington wrote:

> 
> Hi,
> 
> Another go based on Tim's ideas, please comment.
> 
> Notable features: 
>  - set_transient_for virtualized in GtkWindow, so GtkDialog 
>    can override it to destroy the dialog along with 
>    the transient parent
>  - new GtkDialog is totally compatible with old GtkDialog
>    (except that it gets destroyed along with transient parent)

you've already lost source compatibility with changing the
action_area container type. GtkDialog is currently so limited,
that we really shouldn't care about source compatibility that
much, but rather provide a sane API and a comprehensive Changes
log (of course, we can still simply rename the new API like
i did with gtk_aux_dialog(), but aparently you didn't chose
that route).

>  - I didn't support hide-instead-of-destroy-on-close because 
>    this makes things a good bit harder to understand, in 
>    my experience trying to explain GnomeDialog, and also 
>    requires delete_event behavior inconsistent with GtkWindow. 
>    If this is added I think the "close" signal should go in 
>    GtkWindow to keep things consistent, and the "hide on close" 
>    flag should be honored in gtkmain.c for delete_event

uhm, i think part of the confusion here is with what "Close"
actually means. widgets get shown, hidden and destroyed, i think
we shouldn't deviate from that terminology. so you can simply
say:
"Dialogs will be hidden automatically upon activation of a
button, if hide_on_activate is passed as TRUE in
gtk_dialog_add_action_widget().
Dialogs created with GTK_DIALOG_DESTROY_ON_HIDE will be
automatically destroyed when hidden."

this should set things pretty straight for people actually
bothering to read the documentation.
having the dialog only be hidden instead of being destroyed
upon button activation is a fairly important feature that
shouldn't be cut. too many applications already "cache" dialog
creation, and it's also convenient to preserve dialog contents
across multiple invokations (such as the current directory
in a file selection dialog) which is pretty tedious to
implement when having to always go with recreation.

>  - I didn't support a "center over parent window" flag 
>    because I don't think this should be programmer-configurable.
>    However GTK might do it automatically when the transient 
>    parent is set.

i agree that this should depend on the user prefs, still, like
i outlined in my previous mail, you'll want to support popups
at the cursor position to have short lived query dialogs ("Really
Close?") be quickly answered.

> Object arguments aren't implemented yet, I'll do that and also inline
> docs once the design is final.

gtk doesn't actually use inline docs over the place and i don't think
inconsistently adding them in random places is a good idea. instead,
good documentation should explain intention and recommendations of
API usage as well as some implementationbits that may matter.
such stuff belongs into the reference documentation, rather than
cluttering the source files.

> Patch appended, you may want to skip to "Index: gtkdialog.h" because
> diff put the implementation first.

ok, some notes on the implementation.
first, you setup add and remove handlers on the action area,
this is an evil thing, if people actually add widgets
to the action area (e.g. seperators or an alignment), bypassing
gtk_dialog_add_action_widget() (i'd btw s/_widget// for that
function name), they probably know what they do and we shouldn't
fight them with black magic.
instead, simply setup stuff in the adder itself:

void
gtk_dialog_add_action  (GtkDialog *dialog,
                        GtkWidget *widget,
                        gint       action_id,
                        gboolean   hide_on_activate)
{
  ActionData* ad;

  g_return_if_fail (GTK_IS_DIALOG (dialog));
  g_return_if_fail (GTK_IS_WIDGET (widget));
  g_return_if_fail (widget->parent == NULL);
  g_return_if_fail (GTK_WIDGET_GET_CLASS (widget)->activate_signal > 0);
  
  gtk_signal_emit (GTK_OBEJCT (dialog),
                   "add_action",
                   widget,
                   action_id,
                   hide_on_activate);
}

static void
dialog_set_action_id (GtkDialog *dialog,
                      GtkWidget *widget)
{
  dialog->last_action_id = gtk_object_get_data (GTK_OBJECT (widget), "GtkActionID");
  /* alternatively, this could be the default handler for
   * dialog_signals[ACTION] if you want such a signal
   */
}

static void
gtk_dialog_real_add_action (GtkDialog *dialog,
                            GtkWidget *widget,
                            gint       action_id,
                            gboolean   hide_on_activate)
{
  gchar *activate = gtk_signal_name (GTK_WIDGET_GET_CLASS (widget)->activate_signal);
  
  /* "GtkActionID" has to be documented, probably not for
   * GtkDialog only but also for stock widgets, programmers
   * may want to evaluate this as well.
   * if we feature this in other contexts as well (e.g.
   * on the fly menu creation), we might even want to provide
   * accessors for it.
   */
  gtk_object_set_data (GTK_OBJECT (widget),
                       "GtkActionID",
                       GINT_TO_POINTER (action_id));
  gtk_box_pack_end (GTK_BOX (dialog->action_area),
                    widget,
                    FALSE, TRUE, 5);
  gtk_signal_connect_object_while_alive (GTK_OBJECT (widget),
                                         activate,
                                         GTK_SIGNAL_FUNC (dialog_catch_action),
                                         GTK_OBJECT (dialog));
  if (hide_on_activate)
    gtk_signal_connect_object_while_alive (GTK_OBJECT (widget),
                                           activate,
                                           GTK_SIGNAL_FUNC (gtk_widget_hide),
                                           GTK_OBJECT (dialog));
}

also, you're setting up signal handlers on the dialog itself,
gtkdialog.c is a widget implementation, you're supposed to
implement class functions here instead, e.g.:

static gboolean
gtk_dialog_delete_event (GtkWidget *dialog,
                         GtkEvent  *event)
{
  gtk_dialog_hide (dialog);
  
  return TRUE;
}

static void
gtk_dialog_class_init (GtkDialogClass *class)
{
  [...]
  widget_class->delete_event = gtk_dialog_delete_event;
}

your version of gtk_dialog_run() can greatly be simplified,
based on what i mailed earlier in this thread:

gint
gtk_dialog_run (GtkDialog *dialog)
{
  GtkWidget *widget;
  gboolean was_modal;
  gint last_action_id;

  g_return_val_if_fail (GTK_IS_DIALOG (dialog), 0);
  
  widget = GTK_WIDGET (dialog);

  gtk_widget_ref (widget);

  was_modal = GTK_WINDOW (dialog)->modal;
  gtk_window_set_modal (GTK_WINDOW (dialog), TRUE);
  
  dialog->last_action_id = 0;
  gtk_widget_show (widget);
  
  do
    {
      GDK_THREADS_LEAVE ();
      g_main_iteration (TRUE);
      GDK_THREADS_ENTER ();
    }
  while (GTK_WIDGET_VISIBLE (dialog));
  
  last_action_id = dialog->last_action_id;
  gtk_window_set_modal (GTK_WINDOW (dialog), was_modal);

  gtk_widget_unref (widget);
  
  return last_action_id;
}

ok, i think that's enough for now ;)

> 
> Havoc
> 

---
ciaoTJ