Re: [patch] GtkNotebook

[Lars Hamann <hamann braunschweig netsurf de>]

On Sat, Jan 31, 1998 at 01:46:20AM +0100, Tim Janik wrote:

> this is a problem with other widgets as well, from the TODO:
>  * Display of GtkToggleButton is messed up if GtkContainer::container_width
>    is greate than 0. GtkCheckButton and GtkRadioButton are only messed up
>    if draw_indicator is FALSE.

OK, fixed. ;)

 
> > [FIX] (gtk_notebook_page_allocate) :
> >    the tab label is sized too big (by CHILD_SPACING)
> >    e.g. GTK_POS_TOP : 
> >       tab_label->allocation.height was
> >       tab_label->requisition.height + CHILD_SPACING
> 
> shouldn't that be a parameter that can be specified as a widget argument?

I don't know. Would be easy to implement, but I'm not sure if someone
needs it.

> > [FIX] (gtk_notebook_remove_page) :
> >     call gtk_notebook_switch_page if "cur_page" is removed
> 
> this should be done in gtk_notebook_insert_page also,
> actually the signal needs to be emitted in all places where cur_page is
> assigned a new value i.e.
> gtk_notebook_insert_page,
> gtk_notebook_remove_page and
> gtk_notebook_current_page.

OK, gtk_notebook_insert_page should "switch_page", i'll fix this.
But i can't see, why gtk_notebook_current_page should do that ?
  
> also the switch_page signal should be extended to also pass the page number,
> because the notebooks interface i.e.
> void       gtk_notebook_remove_page     (GtkNotebook      *notebook,
>                                          gint              page_num);
> void       gtk_notebook_set_page        (GtkNotebook      *notebook,
>                                          gint              page_num);
> expects page numbers and not page pointers:
>   void (* switch_page)       (GtkNotebook *notebook,
>                               GtkNotebookPage *page,
>                             gint             page_num);

Sounds good.
 
> > Some suggestions :
> > 
> > - How about a popup menu on the tab section for quick page switching ?
> >   This could be done by an additional "gchar *" parameter to 
> >   gtk_notebook_append_page, insert_page, prepend_page. This string will
> >   appear in the popup, describing the contents of the page.
> 
> why couldn't this string just be the notebook label?

Because GtkWidget *tab_label is more powerful. It's easy to use
a box with a Pixmap and Text or something completely different. Maybe
it would even make sense to add another GtkWidget instead of a "gchar *".

> > - As the properties "scrollable", "show_tabs", "show_border" are 
> >   separate styles, how about combining gtk_notebook_set_show_tabs,
> >   set_show_border, set_scrollable into one function gtk_notebook_set_style,
> >   taking an enum as argument ?
> 
> a "widget-style" is already something else, look at gtkwidget.[hc] and
> gtkstyle.[hc] to see what *_set_style is used for.
 
OK, let's say ... gtk_notebook_set_appearance ... or so.
The Question with both suggestions is, if it's OK to break existing code.

> > - Should gtk_notebook_foreach be expanded to also iterate over the
> >   tab label widgets ? These are certainly also children of the
> >   container...
> 
> hm, this issue is a little bit tricky.
> gtkwidget.c relies on certain behaviours on the container_foreach
> functions, and i'm not sure gtknotebook fits that either way...

Could you explain this a little bit ?
I can't see why tab_label should not be processed by 
gtk_widget_set_style_recurse for instance.


bye,
  Lars