After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 115348 - can we port the gnome druid (assistant/wizard like widget) to gtk?
can we port the gnome druid (assistant/wizard like widget) to gtk?
Product: gtk+
Classification: Platform
Component: Widget: Other
Other All
: Normal enhancement
: ---
Assigned To: gtk-bugs
Depends on:
Reported: 2003-06-17 11:33 UTC by Martyn Russell
Modified: 2011-02-04 16:10 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement

gtkassistant.c (42.05 KB, text/plain)
2003-08-18 21:51 UTC, Matthias Clasen
gtkassistant.h (4.86 KB, text/plain)
2003-08-18 21:52 UTC, Matthias Clasen
testassistant.c (19.40 KB, text/plain)
2003-08-18 21:53 UTC, Matthias Clasen
glue (2.91 KB, patch)
2003-08-18 21:54 UTC, Matthias Clasen
needs-work Details | Review
eggassistant.c (17.45 KB, text/plain)
2005-07-12 21:18 UTC, Carlos Garnacho
eggassistant.h (3.34 KB, text/plain)
2005-07-12 21:18 UTC, Carlos Garnacho
eggassistantpage.c (9.75 KB, text/plain)
2005-07-12 21:19 UTC, Carlos Garnacho
eggassistantpage.h (2.79 KB, text/plain)
2005-07-12 21:19 UTC, Carlos Garnacho
sample test (3.11 KB, text/x-csrc)
2005-07-12 21:20 UTC, Carlos Garnacho
first attempt to the proposed solution, .c file (51.44 KB, text/plain)
2005-09-13 13:47 UTC, Carlos Garnacho
first attempt to the proposed solution, .h file (4.50 KB, text/plain)
2005-09-13 13:48 UTC, Carlos Garnacho
some glue (1.61 KB, text/plain)
2005-09-13 13:49 UTC, Carlos Garnacho
tests for the attached implementation (13.59 KB, text/plain)
2005-09-14 12:16 UTC, Carlos Garnacho
second try, .c file (54.44 KB, text/plain)
2005-10-10 16:03 UTC, Carlos Garnacho
second try, .h file (4.71 KB, text/plain)
2005-10-10 16:04 UTC, Carlos Garnacho
c file, now using P_() macro (54.42 KB, text/x-csrc)
2005-12-20 11:01 UTC, Carlos Garnacho
c file, fourth try (56.35 KB, text/x-csrc)
2006-01-14 19:03 UTC, Carlos Garnacho
doc and fixes patch (20.83 KB, patch)
2006-01-24 02:40 UTC, Carlos Garnacho
none Details | Review
patch against current cvs (19.93 KB, patch)
2006-01-24 12:03 UTC, Carlos Garnacho
none Details | Review
another patch (11.40 KB, patch)
2006-01-29 18:35 UTC, Carlos Garnacho
none Details | Review
patch to handle visibility changes, also adds gtk_assistant_update_buttons_state() (2.57 KB, patch)
2006-01-31 12:23 UTC, Carlos Garnacho
none Details | Review

Description Martyn Russell 2003-06-17 11:33:25 UTC
Currently there is no way to do a wizard type function with the widgets
available unless you use a GtkNoteBook using buttons to show and hide tabs
as Next and Previous buttons are clicked.

Gnome already has a druid which does this, can we not port it to GTK, or
create a new widget for this task in GTK+?
Comment 1 Matthias Clasen 2003-08-18 21:51:31 UTC
Created attachment 19323 [details]
Comment 2 Matthias Clasen 2003-08-18 21:52:36 UTC
Created attachment 19324 [details]
Comment 3 Matthias Clasen 2003-08-18 21:53:44 UTC
Created attachment 19325 [details]
Comment 4 Matthias Clasen 2003-08-18 21:54:17 UTC
Created attachment 19326 [details] [review]
Comment 5 Matthias Clasen 2003-08-18 21:56:02 UTC
I've just attached a first attempt at such a port. For a discussion,
Comment 6 Kristof Vansant 2004-11-01 01:15:27 UTC
with linux trying to be more user friendly this looks like a must have. 
Not planned for gtk 2.6? :(
Comment 7 Alan Horkan 2005-04-21 20:28:54 UTC
added the official term "Assistant" into the summary to make this easier to find.  
(I was checking to to see if this would be in the next version of Glade but if
it hasn't made it Gtk+ yet then obviously not)
Comment 8 Carlos Garnacho 2005-07-12 21:17:36 UTC

I'd like to propose the assistant implementation I've done, it has some features
I think they're worth to consider, the implementation is the following:

Inherits from GtkBin, it's just a container which provides title, header pixmap
and watermark, but is not responsible of drawing them. Also provides the
"prepared" signal.

Inherits from GtkDialog, and already provides the buttons and all the frame for
the contained EggAssistantPages. It's responsible of drawing pages' title,
header or watermark. For drawing the top and side colors it just inverses the
GTK_STATE_NORMAL and GTK_STATE_SELECTED colors, thus adapting to theme changes.

It uses egg_assistant_set_forward_page_func() and
egg_assistant_set_back_page_func() for handling page switches instead of "next"
and "back" signals. By default, the forward function moves to the next page, and
back function returns to the previously shown page, independently of the forward
function (maintains a list of visited pages), so unless people want to make very
crazy things, this is a very sane default.

In general terms, The API to handle pages is quite similar to the GtkNotebook one.

issues left
- color drawing doesn't work with pixmap themes (but this isn't specific to this
- inverted color still doesn't play fine with unsensitive assistants
- maybe it could be worth adding a method in EggAssistant for handling watermark
alignment, like in Matthias' implementation

I'd thank any comment on this implementation :)
Comment 9 Carlos Garnacho 2005-07-12 21:18:26 UTC
Created attachment 49059 [details]
Comment 10 Carlos Garnacho 2005-07-12 21:18:58 UTC
Created attachment 49060 [details]
Comment 11 Carlos Garnacho 2005-07-12 21:19:28 UTC
Created attachment 49061 [details]
Comment 12 Carlos Garnacho 2005-07-12 21:19:57 UTC
Created attachment 49062 [details]
Comment 13 Carlos Garnacho 2005-07-12 21:20:43 UTC
Created attachment 49063 [details]
sample test
Comment 14 Matthias Clasen 2005-09-03 03:23:19 UTC
Hmm, after looking over the api and implementation, I have to admit that I still
prefer my older proposal, which gets rid of the separate page objects, and does
not use a notebook internally.
Comment 15 Carlos Garnacho 2005-09-03 16:35:46 UTC

I see 2 different concepts here:

1) Global header and side image, possibly handle changes through the "prepare"
2) per page images, setting it is implicit in the GtkAssistant code

I did 2) because (as far as I've seen) it's quite usual to change those images
depending on the assistant stage, and as you may notice, the GtkAssistantPage
widget is a really thin layer to provide that per-page data.

another solution could be to attach that data internally to the inserted
widgets, so we can get rid of GtkAssistantPage, but I don't know how well would
this get along with Glade.

Regarding the GtkNotebook use, after implementing my own code to draw pages and
so, I felt that it could be a lot cleaner and less error prone if I used a
widget that already handles that
Comment 16 Matthias Clasen 2005-09-03 17:57:32 UTC
carlos, look how my older code does the page titles. Child properties are the
GTK+ mechanism for things like this. I think the images should be handled in
that way, too. 

So, after thinking some more about the different approaches here, I think I want
to try and merge the best aspects of both patches. 

- no page objects in the api, use child properties for title, image, etc
- ditch the "forward" and "back" signals, and use your approach of specifying
  a function to compute the next page.
- derive the assistant from  GtkWindow (I don't think the GtkDialog response
  API is a very good match for an assistant)
- use a stack of visited pages as you do to implement "back". I don't think 
  there is a need to specify a custom back function, it should always go back 
  to the previous page
- keep the page types of my patch
- add a per-page boolean property "complete", which indicates whether all 
  required fields on the page have been filled in

I think by combining the "forward" function, the page types and the "complete"
information, we can do a very good job with flow control. E.g if all pages
following the current page are already marked complete (because they only
contain optional fields), we can show a "finish" button (if the sequence of
following pages ends in a page of type "confirm", we would show a "last" button
instead, which would jump to the confirm page).
Comment 17 Matthias Clasen 2005-09-03 18:03:04 UTC
To explain a bit more, the idea of the "complete" property is that you don't set
the sensitivity of the buttons directly, but instead mark a page as complete
when it is sufficently filled in, and let the assistant figure out how that
translates into visibility and sensitivity of buttons. 

Oh, and of course we should keep the possibility to hide all standard buttons in
the prepare signal, and add some way to add custom widgets to the action area.
Such custom widgets can be used e.g. to show a progressbar if some activity on
the page takes a long time, or to add custom navigation buttons, if that should
be possible. (To enable custom navigation buttons, we need a set_current_page()
Comment 18 Carlos Garnacho 2005-09-03 20:03:11 UTC
Nice, I didn't know a lot about child properties, thanks for pointing that out :)

Regarding the rest of the proposal, it looks quite good (reimplementing
action_area for GtkAssistant sounds like duplicate work, but I may understand
your GtkDialog inheritance point), 

Do you plan to implement it? or may I work on this?
Comment 19 Matthias Clasen 2005-09-04 15:48:53 UTC
reimplementing action_area could be as simple as adding

gtk_assistant_{add,remove}_action_widget (GtkAssistant *assistant, 
                                          GtkWidget    *widget)

Sure, you can work on it if you want. Otherwise I will revisit it in
approx. 2 weeks. If you want to look into it, let me point out a few more
things I noticed in your first implementation:

- we still need an "apply" signal (this was called "finish" in the libgnomeui
druid), so that the application knows when to act on the data collected by the
wizard. Its probably a good idea to also keep the "cancel" and "close" signals.
Note that pressing the "finish" button in a simple assistant should probably 
emit both "apply" and "close", since more complicated assistants may have a
summary page (and possibly also a progress page) after the one with the "finish"

- the list of visited pages must be emptied when the assistant is done. "unmap"
might be good place to do that.

- a destroy notify is needed for the data registered with the forward_page_func

There were some more things I noticed, but I forgot to write then down...
Comment 20 Carlos Garnacho 2005-09-04 18:15:25 UTC
Nice, I'll begin working on it tonight, thanks for the new comments, and don't
forget to drop here your feedback :)

Regarding the "apply" signal emission, I was handling it diferently, as the
widget inherited from GtkDialog, I thought that people would do something like:

GtkAssistant *assistant = get_assistant ();

if (gtk_dialog_run (GTK_DIALOG (assistant)) == GTK_RESPONSE_APPLY)
  apply_changes (assistant);

but anyway, emitting an "apply" signal would have been good too...
Comment 21 Matthias Clasen 2005-09-04 21:07:28 UTC

> if (gtk_dialog_run (GTK_DIALOG (assistant)) == GTK_RESPONSE_APPLY)
>   apply_changes (assistant);

Thats actually one of the reasons why I think the response api is not
a good fit for the assistant. There are cases where you want to apply the
collected data before unmapping the dialog. If applying the data takes 
a long time, you might swich to another page displaying a progress bar,
and once the changes are complete, swith to a summary page which lists
what has been changed. The summary page only has a close button. (all this
is handled by the page type api in my proposal)
Comment 22 Carlos Garnacho 2005-09-13 13:47:43 UTC
Created attachment 52170 [details]
first attempt to the proposed solution, .c file
Comment 23 Carlos Garnacho 2005-09-13 13:48:31 UTC
Created attachment 52171 [details]
first attempt to the proposed solution, .h file
Comment 24 Carlos Garnacho 2005-09-13 13:49:14 UTC
Created attachment 52172 [details]
some glue
Comment 25 Carlos Garnacho 2005-09-14 12:16:16 UTC
Created attachment 52221 [details]
tests for the attached implementation
Comment 26 Matthias Clasen 2005-10-05 19:42:28 UTC
Feedback from reading the files (I haven't actually tried it yet):

I think we prefer to put enumerations in the same header file as the class they
belong to, so I think GtkAssistantPageType should be in gtkassistant.h (I notice
my earlier code got that wrong too).

> gint         gtk_assistant_insert_page      (GtkAssistant *assistant,
GtkWidget *sibling, gint position);

The second parameter should probably be named "page", not "sibling"

>    _("Whether the page contents are correctly filled"),

Maybe better "Whether all required fields on the page have been filled"
It would probably be good to add a longer doc comment for this property
and explain that the property should be set to TRUE for pages which have
no required fields and point out that the value of the property is
used to control the visibility and sensitivity of the "Last" button (if
we add support for a "Last" button, that is)

> static gint
> default_forward_function (gint current_page, gpointer data)
> {
>   return ++current_page;
> }

I think the default forward function should take page visibility into
account, and skip invisible pages. Or is that taken care of somewhere else ?

The switch statement in _set_assistant_buttons_state() needs a break for
the last case, and just to be safe, a default: g_assert_not_reached(); case.

In the else branch in compute_next_step(), don't you miss an

if (do_apply) 

there ? Why is the apply signal emitted from compute_next_step(), for that
matter ? Wouldn't it be cleaner to emit it from on_assistant_apply() ?

_remove_assistant_page() needs to deal with removing a page that has already
been visited, I think.

>   /* if there's no default page, pick the first one */

Should take visibility into account here, too.

gtk_assistant_forall() needs to include the buttons when include_internals

More when I have tried the examples...
Comment 27 Carlos Garnacho 2005-10-10 16:03:49 UTC
Created attachment 53304 [details]
second try, .c file
Comment 28 Carlos Garnacho 2005-10-10 16:04:24 UTC
Created attachment 53305 [details]
second try, .h file
Comment 29 Carlos Garnacho 2005-10-10 16:11:15 UTC
the new patches fixes those issues, and implement the "Last" button (plus other
bugs/memleaks fixing), I've got some answers to your comments:

- The compute_next_step() signals emission was mostly for handling more or less
elegantly assistants with a broken flow (i.e.: not ending in a page of type
"confirm" or "summary"), now I just throw a g_critical() in suck cases, is that ok?

- the assistant buttons are children of priv->action_area, which is handled in
Comment 30 Filip Van Raemdonck 2005-12-20 10:03:34 UTC
The latest files appear to be missing a definition for GTK_TYPE_ASSISTANT_PAGE_TYPE (and from that, an implementation probably for a gtk_assistant_page_type_get_type() method).

Also, it includes gtk/gtkmarshalers.h but does not seem to use it anywhere. And that #include is not available when trying to use it outside of GTK+ compilation (as an EggAssistant like thingy).
Similarly gtk/gtkintl.h is not available outside GTK+ either, but I am not sure what the policy would be regarding inclusion of glib/gi18n.h instead.
Comment 31 Carlos Garnacho 2005-12-20 10:55:22 UTC
The last implementation doesn't need a GtkAssistantPage type, it uses widgets and child properties as pages. 

The inclussion of gtkmarshalers.h should probably be removed (unless there is some obscure reason for using _gtk_marshal_* instead of g_cclosure_marshal_* I haven't seen), but the use of gtkintl.h is necessary for using the P_() macro for properties (which I've realized I havent used)
Comment 32 Carlos Garnacho 2005-12-20 11:01:37 UTC
Created attachment 56197 [details]
c file, now using P_() macro
Comment 33 Filip Van Raemdonck 2005-12-20 12:21:49 UTC
 "The last implementation doesn't need a GtkAssistantPage type, it uses widgets and child properties as pages."

... and uses a GTK_TYPE_ASSISTANT_PAGE_TYPE macro when adding these properties, which macro was not available anywhere.

I now realize this is because it is likely generated if you build this inside GTK+ with the necessary glue; I tried using it as a libegg alike drop-in in an application. I generated the glue code myself now.

Something else: I noticed that the page titles are top-aligned with the (top header) logo, whereas GnomeDruid used to center-align them alongside each other. IMO the center alignment looked better ;-)

And the assistant does not seem to accommodate for page title length when calculating window size, which means that if the second (or a subsequent page) is larger than the first it gets wider when clicking next. GnomeDruid handled this ok, too - it calculated the maximum required width in advance.
Comment 34 Carlos Garnacho 2006-01-14 19:03:26 UTC
Created attachment 57356 [details]
c file, fourth try

Oops, sorry, I thought you were referring to a GtkAssistantPage GType.

The new C file takes into account page title and header image sizes during size allocation, uses cairo for drawing the colored box, adds the "content_padding" style property and adds some minor improvements to the code
Comment 35 Matthias Clasen 2006-01-18 22:38:27 UTC
I have committed this now, it seems good enough, and we can fix remaining
small issues in-tree. A few things still need to be done:

- Add some GtkAssistant examples to gtk-demo, add a standalone

- Flesh out docs, add some examples

- Write a porting guide for porting from GnomeDruid to GtkAssistant
Comment 36 Matthias Clasen 2006-01-18 23:38:38 UTC
Detailing this a bit more, I think the tests in #25 are a great starting point
for a comprehensive testassistant. Possible additions: 

- add some invisible pages, to test that they are properly skipped
- add a page with a sidebar image
- test "complete" handling, by adding a page with an entry which
  must be filled out
- a progress page with a progressbar
- add a custom page, where header and sidebar are not used, and the
  page itself contains some decorations
- test a custom page forward function

For gtk-demo, we should probably just add one somewhat representative
assistant, which has some bells and whistles, without being excessively
Comment 37 J. Ali Harlow 2006-01-19 13:29:05 UTC
I see that gtkassistant.h (unlike the C file) was committed with DOS-style CR/LF pairs. Not sure if this matters...
Comment 38 Matthias Clasen 2006-01-19 13:41:48 UTC
Should be fixed
Comment 39 J. Ali Harlow 2006-01-20 18:40:29 UTC
In trying to use GtkAssistant in a new application, I found the following issues:

* Child property CHILD_PROP_PAGE_TYPE is named inconsistently:

        Enum value                      Name
        CHILD_PROP_PAGE_TYPE            page-type
        CHILD_PROP_PAGE_TITLE           title
        CHILD_PROP_PAGE_HEADER_IMAGE    header-image
        CHILD_PROP_PAGE_SIDEBAR_IMAGE   sidebar-image
        CHILD_PROP_PAGE_COMPLETE        complete

  Should probably be simply "title".

* Last button is misnamed under some circumstances:

        Page    Type            Contents        Last button
        0       Intro           Finding modems  -
        1       Content (c)     Port (defaults) Will skip to page 3
        2       Content (c)     Type (defaults) -
        3       Progress        Detecting...    -
        4       Content         Modem specific  -
        5       Progress        Configuring...  -
        6       Summary         Done            -

        (c) Page is marked as complete

  Instead, we need something like "_Jump forward" (with jump-to icon?)
  where the destination page isn't the last page.

* Cancel, apply and close signals are documented as passing a page parameter,   but don't.

* It would be very useful if summary pages could be made to display the
  back button to allow users to change their minds where appropriate
  (rather than having to close the assistant and re-start it from the
  beginning as at present).
Comment 40 J. Ali Harlow 2006-01-20 18:44:37 UTC
be simply "type", is of course what I meant to say.
Comment 41 Matthias Clasen 2006-01-20 21:21:44 UTC
Well, page-type is consistant with GtkAssistantPageType. What is inconsistent
here is probably that the PROP constants for the other properties all have 
PAGE in their name, but I don't think its something to worry about.

Regarding Jump, maybe. I'm not sure if it would be confusing. Wouldn't people
wonder what steps they are skipping. The idea with the Last button was that
you organize your wizard so that all optional steps go at the end, and then
offer an early exit via the Last button.  Maybe we should display a jump button 
in situations like you described, but then we probably need to add a way
to turn it off.

We should probably fix the signals to behave as specified.

The idea of the Summary page is that it comes after applying whatever settings
the wizard collected. So the damage is already done at that point. If you want 
to allow the user to inspect the collected settings before deciding to apply them or go back and make changes, use a confirm page.
Comment 42 Matthias Clasen 2006-01-21 06:26:23 UTC
I have fixed the signal docs, and added the testcases from #13 as
Comment 43 J. Ali Harlow 2006-01-21 12:40:28 UTC
Re: Jump-Forward, I certainly accept that it isn't perfect but I was working from the assumption that the current code to decide when the "last" button should be displayed was correct and that just changing the button name & icon would be an improvement. If we follow your argument in comment #41, then we shouldn't display any button at all in these cases. Ie., we stick with just the "last" button and change the check to display from (count > 1) to (count > 1 && page_num < 0). I'm just as happy with this, if you prefer it.

Re: Summary page. Granted the damage has already been done, but the user may well only realise on reading the summary page that they've got it wrong and want to redo the operation. This is semantically equivalent to the undo menu item which many applications go to great lengths to provide. I would certainly agree that a back button may not be appropriate for all assistants, but it would be nice if we could allow applications to provide this functionality if it makes sense.
Comment 44 J. Ali Harlow 2006-01-23 16:29:28 UTC
Another minor niggle I've just noticed with the last button is that its visibility depends on the forward-chain. If an application sets its own forward page function which varies the chain depending on the values entered by the user then there is no mechanism in place for the application to tell the assistant that the forward chain has been changed so that it can re-calculate whether to show or hide the last button.

Perhaps we need a gtk_assistant_queue_rechain() or something?
Comment 45 Matthias Clasen 2006-01-23 17:33:20 UTC
A user-specified forward function should be prepared to calculate the next page for every page based, on the current values. For "future" pages, that would presumably be based on the default values. 
Comment 46 Matthias Clasen 2006-01-23 17:37:27 UTC
A small problem with current compute_last_button_state implementation is that it
does not check for cycles.
Comment 47 J. Ali Harlow 2006-01-23 17:55:36 UTC
Re: Comment #45 which I assume is a reply to comment #44:

That's all true, but it doesn't really help. Consider the code in ::set_complete() that is designed to cope with the complete property changing value on either the current page or a "future" page which might require that the last button be either hidden or shown. If it's right for the assistant to track changes to complete that affect the visibility of the last button, then why isn't it also right that it should track changes in the forward chain which have the same effect? Since it's not possible for the assistant to track such changes without help from the application, we need to provide a means by which this can be done.
Comment 48 Matthias Clasen 2006-01-23 19:24:37 UTC
As far as I can see, the assistant code determines the visibility of the last button when the completeness of any page changes. It should probably also track the visibility of pages. Oh, but you are worrying about cases where the user
changes a value on a page which might cause the forward chain to change, making it necessary for the last button to change visibility. Yes, we don't handle that currently, and you are right that it might be simplest to have some recompute()
function for such cases.

I also notice another fine point of last button handling we don't currently get right. If all future pages are complete, but the current page is not yet, we should show the last button, but keep it insensitive until the current page is complete.
Comment 49 J. Ali Harlow 2006-01-23 20:26:02 UTC
If the assistant is supposed to be able to cope with pages changing visibility, then yes, we need to track this too. I had originally altered the visibility of pages to modify the forward chain rather than using a forward page function, but I found that while calling gtk_widget_hide() wasn't a problem, calling gtk_widget_show() caused widgets on the "future" pages to be mapped instantly. If this is supposed to work, then gtkassistant!_set_current_page() probably needs to call gtk_widget_set_child_visible() to make this work.

This at least is a nice easy bug with no decisions to be made. I'll start working on a patch tomorrow. I might do the tracking of page visibility also.
Comment 50 Matthias Clasen 2006-01-23 22:45:36 UTC
good catch. 
Comment 51 Carlos Garnacho 2006-01-24 02:40:37 UTC
Created attachment 57973 [details] [review]
doc and fixes patch

Hello, finally had some time to work on this patch, here's the list of changes:

- Adds a doc chapter about porting from GnomeDruid to GtkAssistant
- Adds an assistant sample to gtk-demo
- Adds keyboard focus handling (sorry, didn't notice this before)
- Improves some documentation and fixes some typos
- Fixes a possible crash produced when there aren't more visible pages in the assistant flow
- Avoids cicles in compute_last_button_state, and makes it to show the "last" button insensitive when the current page isn't complete (as suggested in #48)
- Does a couple of minor improvements

As far as I can see, ATM there are only left the issues commented in #44 and #49, a patch would be great for these, if not, I can work on them
Comment 52 Matthias Clasen 2006-01-24 06:07:34 UTC
Great work. You may have to redo the patch against current cvs though, since I made some changes to gtkassistant.c today, and also fixed the looping problem in 
compute_last_button_state() in a different way.
Comment 53 Carlos Garnacho 2006-01-24 12:03:44 UTC
Created attachment 58008 [details] [review]
patch against current cvs
Comment 54 J. Ali Harlow 2006-01-24 12:17:21 UTC
Carlos, please be my guest. It's not as if I don't have other things to do :)

Can I raise another point: We often want to run quite lengthly processes in
assistants (sometimes several minutes). I presume these should be implemented
using progress pages in the new API. However, neither the behaviour of
incomplete progress pages (where cancel, back and forward buttons are
insensitive) nor of complete progress pages (where all three are sensitive)
seems to meet our needs.

I can envision a situation where the current implementation of complete makes
sense - if you have a process that is optional and can by skipped by the user
(eg., a delay or polling for something that the user knows will never be

However, where the process can't be skipped, I can't see what rational there is
for concluding that it also can't be cancelled. Indeed the HIG (section 7 -
Allowing Interruptions) would seem to preclude the current behaviour.

I suggest that the back and cancel buttons always be sensitibe for progress
pages. It would be nice if we could follow the HIG's recommendation further and
provide some means to replace cancel with stop if this is more appropriate,
perhaps most easily implemented with a new page type.
Comment 55 Matthias Clasen 2006-01-24 14:21:59 UTC
all we are discussing here is only default behaviour anyway. If you need to, 
you can frob the visibility/sensitivity of the buttons in the prepare handler.
That is supposed to work. But lets discuss the default behaviour. Your proposal
is to 

- make back and cancel sensitive by default on progress pages
- treat progress pages like content pages wrt to last button handling
- add an optional stop button on progress pages that would stop
  the currently progressing activity without canceling the assistant.

For that we probably need a stop signal. If a progress page is stoppable, 
we could also allow the user to use the Forward button to stop the
action and move to the next page. But maybe it is clearer to only make
the stop button sensitive while the action is running, and enable
cancel/back/forward when the action is stopped or complete.

Comments ?
Comment 56 J. Ali Harlow 2006-01-24 15:06:55 UTC
Ah, I hadn't understood that such frobbing was supported. Indeed, I've only just realized that the buttons are available as public members of GtkAssistant. Very nice. Presumably we'll need to take steps in due course to prevent buttons flashing up if they would be visible by default but the prepare handler is used to hide them.

The first part of the proposal listed is certainly mine. I'm not clear where the second part come from. Perhaps it's implied by something I've said, but I'm missing it at the momemnt.

Regarding a stop button, the HIG recommends that this button is used instead of cancel and should have a very similar meaning except that the operation in progress can't be backed out of. The application is supposed to put up an alert detailing the options available to the user. On this basis, I think it would be better if the assistant acted just as if it were a cancel button (ie., generate the cancel signal plus the WM close handling) and leave it to the application to take whatever action if necessary. Whether it is worth supporting the switch from cancel to stop in the assistant or whether applications should just use the prepare handler to hide the cancel button and add the stop button as an action widget, I'm not clear. One advantage of doing it inside the assistant is the buttons will appear in the right order.

I'm not a usability guy, but from what I understand of the HIG, normal progress windows are supposed to automatically close once the operation is completed (the exception being checklist windows). thus I usually arrange that assistant progress pages are only shown while the operation is in progress and automatically advance to the next page once it's done. This allows me to think of "back" as meaning stop the operation and let me re-consider (after which I can use "forward" to mean start the operation again) and "forward" as meaning skip the rest of the operation and continue.

For me, requiring that I stop the operation and then go back (ie., two seperate steps) just complicates things.
Comment 57 Carlos Garnacho 2006-01-24 15:24:25 UTC
(In reply to comment #55)
> - make back and cancel sensitive by default on progress pages
> - treat progress pages like content pages wrt to last button handling
> - add an optional stop button on progress pages that would stop
>   the currently progressing activity without canceling the assistant.
> For that we probably need a stop signal. If a progress page is stoppable, 
> we could also allow the user to use the Forward button to stop the
> action and move to the next page. But maybe it is clearer to only make
> the stop button sensitive while the action is running, and enable
> cancel/back/forward when the action is stopped or complete.
> Comments ?

hmmm, IMHO an "stop" signal is quite out of the scope of an assistant widget, the current code works fine for the case of non-stoppable actions, and for the other case it adds other features:

- buttons as public variables
- gtk_assistant_add_action_widget () to add other buttons to the action area

with the current behavior we cover the most common case, but the widget is lax enough to let developers handle the other one without big worries
Comment 58 Matthias Clasen 2006-01-28 06:12:37 UTC
I have committed the last patch. Here is my current TODO list for GtkAssistant:

- implement visibility handling (see #48)
- add long description 
- add some more test (see #36)

Comment 59 Carlos Garnacho 2006-01-29 18:35:51 UTC
Created attachment 58340 [details] [review]
another patch

This is the summary of changes:

- Adds migrating-GtkAssistant.sgml to in the gtk reference directory, it seems it was missing.
- Adds long description to docs, also documents GtkAssistantPageFunc and GtkAssistantPageType. With these, I think the API is 100% covered.
- Adds GtkAssistant to doc-shooter
- implements children visibility handling
- fixes assistant_paint_colored_box() to take container border width into account
- adds another test to testassistant to use all untested features (hidden pages handling, sidebar and header images and widgets in the action area)
Comment 60 Matthias Clasen 2006-01-30 05:08:23 UTC
Thanks for the patch, I have committed it. 

I think we need to watch out for changes in the visibility of pages, and recalculate button state in that case. It has also been mentioned before that it might be a good idea to have a function to manually trigger a recalculation 
of the button state (user input on page A may influence the page sequence ahead
of A, requiring an update of the visibility/sensitivity of the last button while
the user interacts with page A)

Also, did you see the bug about HIG compliant spacing etc ? 
Comment 61 Matthias Clasen 2006-01-30 14:32:59 UTC
bug 328082 is the one about spacing issues. 
Comment 62 Carlos Garnacho 2006-01-31 12:23:44 UTC
Created attachment 58464 [details] [review]
patch to handle visibility changes, also adds gtk_assistant_update_buttons_state()
Comment 63 Matthias Clasen 2006-01-31 17:00:29 UTC
2006-01-31  Matthias Clasen  <>

	* gtk/gtk.symbols:
	* gtk/gtkassistant.h: 
	* gtk/gtkassistant.c: Actually implement visibility handling,
	and add gtk_assistant_update_buttons_state.

	* tests/testassistant.c: Test visibility handling.
Comment 64 Matthias Clasen 2006-01-31 17:02:09 UTC
I think we are ready to close this now. 
Please open a new bug if I overlooked something.