Re: dialog enhancements

[Havoc Pennington <hp redhat com>]

Tim Janik <timj@gtk.org> writes: 
> ok, i've spent quite some time composing this email, looking at your header,
> GnomeDialog and other things, and tried to provide constructve criticism.
> but this also lead me to point out some general mistakes in other people's
> code/APIs, so please don't take my comments personally ;)
> 

Of course not, your API proposed at the end looks promising. (I did
not like the Beast one though, I did look at it as you suggested.)

> 
> > Here's what I added:
> >  - changed the action_area to be a button box
> 
> urg, button box is one of the widgets with the worst packing behaviours
> i've come across, for gtk proper, we should better stay HBox and let gnome
> libraries override that if required.
> 

If you don't like the way button box packs buttons, why don't we just
change it so it's nice?

It's dense to have a button box and not use it for buttons. ;-)
Certainly if it's wrong for GTK it is also wrong for GNOME.

Anyway, what's wrong with it? 

 
> this is an inherently bad idea, and it is what makes usage of quite
> some widgets, such as the option menu and several gnome ones very ugly
> and tedious.

I think you're getting stuck on the word "position". Perhaps I should
s/position/action_id/ and you would be happy. ;-) They are just unique
integers whose exact value doesn't matter much. The only difference is
with your action_id the user can specify the ID. Of course the
disadvantage is that the user _must_ specify the ID. 

So if we add a convenience API, such as a varargs list of buttons
labels, it would make sense to automatically generate the action_id
for those buttons, otherwise it isn't so convenient.

> 1) "position" in itself is dubious, this can be the position within the
>    the containers children list, or the "position" of the widget within
>    its siblings on the screen. this is not necessarily the same, think
>    of hidden siblings

But who cares, this situation never happens. Who is going to hide a
dialog button? That would be broken. 

> 2) a position is an extremely dynamic thing, it changes when siblings
>    are inserted/removed, and may be totaly bogus when the user interface
>    doesn't reflect the order in which children got added.

Inserting/removing buttons dynamically does not happen in any real
application. In a GUI builder yes, but it doesn't matter there.

> 3) a position is a relative reference only applicable within context,
>    that is, you can only make use of it, only if you also have the context
>    (dialog) pointer around that this position referes to, i.e. you can't
>    pass it in to some signal handler that can then pass it on into
>    gtk_widget_set_sensitive() or similar functions

Sure, but this is also not something you need to do 99% of the
time. ;-) 

Don't spend too much time thinking about the corner cases - remember
that anyone can make their own dialogs from scratch with GtkWindow if
they want. This whole widget is purely a convenience
thing. Convenience things that aren't convenient are just bloat.

However I will grant you your points about position, since an
action_id will work just as well as you suggest.

> this is why Gtk+ throughout most of its interface only exports widget
> pointers. you can use them irregardless of context, and they remain
> valid upon alterations of other things, such as dynamic layout changes.
> some people seem to have taken a dislike for pointers in general,
> which is probably why they prefer half-backed more-natural-looking?
> widget references, such as integers. but fact is, this is still C we
> code in, and there is only one authoritative reference to a thing
> in C, and that is its memory adress, read: pointer.
>

People don't like pointers because you can't map them to actions
easily.

For example:
 
GtkWidget* clicked_button;

clicked_button = gtk_dialog_run(dialog);

/* Now what? How do I know what button I have? */

Basically you have to set_data() an integer ID or a string ID or
something so you know what to do with this button. The pointer by
itself is useless here. Thus the position or action_id is very nice to
have.

(And a similar position or action_id interface would be quite nice for
GtkOptionMenu, by the way... people request this feature very often.)
  
> i pretty dislike GnomeDialog's varargs interfaces, and your's isn't
> much better to be honest. for one, varargs don't necessarily make
> things easier for people, or we'd have more applications actually
> using the gtk_widget_new() and gtk_widget_set() interfaces of gtk.

In this case the varargs are just saving you typing. 

Compare:
 dialog = gtk_dialog_new("Title", parent, "OK", "Cancel", NULL);

and:
 dialog = gtk_dialog_new("Title", parent);
 gtk_dialog_append_button(dialog, "OK");
 gtk_dialog_append_button(dialog, "Cancel");

or worse:
 dialog = gtk_dialog_new("Title", parent);
 gtk_dialog_append_widget(dialog,
                          gtk_button_new_with_label("OK"));
 gtk_dialog_append_widget(dialog,
                          gtk_button_new_with_label("Cancel"));

98% of dialogs have only buttons in the action area, so this last one
is very gratuitous pain. 

> for another, if you decide to take the route of offering a varargs
> interface, that can be much more powerfull than simply being
> constrained to a NULL terminated string list, as an example, here's
> a dialog widget from BEAST that creates buttons on and connects
> signals on the fly:
> 

Yes, but that is too complicated to be a convenience interface. No one
would ever use it (much as they don't use gtk_widget_new).
 
> amongst other things that really aren't interesting for the moment,
> this function call creates a dialog that comes with an "Execute"
> button, having two functions connected to it, and a "Close" button,
> that also has a signal connection. additionally, "Execute" is made
> the default choice of the buttons.

(This is sort of irrelevant, but I'm wondering how you call gettext()
on your button labels there.)

> stock items are a candidate for this, but we better figure out an
> extensible system first to deal with stock pixmaps, widgets,
> buttons, internationalizable labels, menu items etc. and user
> preference settings of somesuch, that GNOME can incrementally
> integrate with, before exposing "stock concept" as defines in
> some widget header file.
>

Indeed, I agree. I just wanted to illustrate the concept.
 
> what's the va_list variant for? are there really going to be functions
> that carry on the label list varargs thingy, but are too sophisticated
> to call gtk_dialog_append_button() on their own then? ;)
> this tastes like a bunch of the cruft which went into GnomeDialog,
> that tries to provide some pretend-to-be-convenience functions for
> language bindings, where language bindings better make sure to cook up
> their language-specific convenience on their own and use normal gtk
> interfaces to set things up.
> 

OK, just trying to make the bindings easier. 
 
> > /* emit button_clicked signal */
> > void       gtk_dialog_button_clicked             (GtkDialog   *dialog,
> >                                                   gint         button);
> 
> what exactly is this usefull for? i'm not saying it is not usefully,
> just wondering what people would use this for (since then we might
> want to have gtk_aux_dialog_activate () function in the below as well).
>

People use it to force a certain return value from gtk_dialog_run(),
basically.

You could even call it gtk_dialog_quit_run(dialog, return_code); 
 
> dialogs are used for a wide of variety things, some of them simply display
> a message, others are used to make a yes/no or 1-out-of-many choice,
> and they are also frequently used for short hand inputs (e.g. changing
> a layer name in gimp's layers&channels dialog).
>

I think you want to make some assumptions in the convenience interface.

1) All dialogs have buttons on the bottom; if they don't, they are
   probably a toolbox or something. for example, layers&channels is 
   not a dialog in my opinion, it is just a small auxiliary window.

   People should use GtkWindow for persistent stuff like
   layers&channels. Your GtkDialog interface should be for transient
   dialogs that pop up for a short time, the user quickly clicks some
   stuff, the dialog pops down.
   
2) The most common dialog by far is a simple message with an OK
   button.  Thus the MessageDialog convenience object. 

> in order to come up with a dialog API that is generically usable, we
> have

Remember though: GtkWindow is the generic interface. GtkDialog is
_only_ a convenience interface - that's its only reason for existing,
at all. So, I think it's important to be as generic as possible, while
primarily being convenient, instead of being as convenient as
possible, while primarily being generic.

> - title settings; it is very annoying to have a dialog popped up that comes
>   with now title/only the application name as title (what is done if no title
>   is set is window manager dependant)

Constructor should require this I think.

> - positioning; i.e. short lived dialogs that the user can quickly fill in or
>   answer should be made easily accessible, e.g. by popping them up at the
>   current mouse cursor position

I think GTK+ should automatically handle this, considering the
transient parent, and perhaps honoring user preferences settings.

> - lifetime specification; this is the decision of creating a dialog on demand
>   and destroying it upon hiding, or keeping it around for later
> reuse

GnomeDialog has this "hide instead of destroy" feature, but I can't
actually think of a case where it's more convenient. Can you think of
such a case?

> - dialog parentship; a bunch of temporary dialogs belong to other longer-lived
>   toplevels (e.g. the "Really close this window? Y/N" type of dialogs), and
>   shouldn't stay around longer than the toplevel they refer to

Good point, transients should perhaps be destroyed when their parent
is destroyed.

> - pointer/memory maintenance; some apps don't keep track of dialog invokation,
>   which can lead to multiple dialogs of the same kind being popped up (e.g.
>   the above "Really close this window? Y/N" type). some applications that do
>   try to keep track get the destruction notification wrong (or don't bother),
>   or attempt to use reference counting (which is utterly wrong for this case)
> 

Good.

> GtkWidget* gtk_aux_dialog_new (GtkWidget        *context_parent,
>                                const gchar      *dialog_title
>                                GtkWidget        *content_child,
>                                GtkAuxDialogFlags flags,
>                                GtkWidget       **aux_dialog_p);
>

I really want varargs button labels on the end of this. ;-)
Maybe with action IDs alternating with the labels.

Then I can create a useful dialog in a single function call.

Otherwise, I have to call some other functions to add my buttons.

> void gtk_aux_dialog_add_action (GtkAuxDialog *aux_dialog,
>                                 GtkWidget    *action_child,
>                                 gboolean      hide_dialog_on_activate,
>                                 gint          action_id);
> 

Consider that to make action_id better than position, it needs to
always be an enumeration. If it's just random integers, it's no better
than position.

Writing an enumeration per dialog is pretty inconvenient.

One nice solution might be to provide some stock action_id values:
 
typedef enum {
 GTK_ACTION_ACCEPT,
 GTK_ACTION_REJECT
} GtkActionsType;

One of the nicest things about action_id is that you can have
duplicates, so two buttons can do the same thing.

> /* gtk_dialog_run() shows the dialog and enters a recursive main loop
>  * to be executed until the dialog is hidden. the return value is the
>  * action_id of the child that was lastly activated or 0 if none.
>  * gtk_dialog_run() is recommended to be used only with
>  * GTK_AUX_DIALOG_MODAL dialogs.

gnome_dialog_run() actually makes the dialog modal for the duration of
the run - I guess it is almost impossible to use this function sanely
if the dialog is not modal.

> a few notes on the implementation, gtk_aux_dialog_add_action() should
> for instance be implemented by a signal of GtkAuxDialogClass, so GNOME
> libraries can override behaviour to specify policy, e.g. by altering
> packing of the action widgets.
>

I think we should just have nice packing in GTK also - but we can
virtualize the function too. 
 
> on top of that foundation, we can then build convenience functions
> which cover needs like automated printf() style content creation.
> things like on-the-fly creation of sub-heirachies, such as a button
> containing a label, better be implemented on a per container-basis,
> here, e.g. gtkbutton.h, so they are then generically pluggable into
> functions like container_add() or aux_dialog_add_action().
> 

Here I think gtk_button_new_with_label() is too much typing for 
dialog buttons. 

I do like your proposed API though. I will do a revised GtkDialog
based on it and post a new draft.

Havoc