Re: GtkExpander/GtkDisclosure

[Mark McLoughlin <mark skynet ie>]

Hi Owen,

On Fri, 2003-03-14 at 04:20, Owen Taylor wrote:
> On Wed, 2003-03-12 at 18:45, Mark McLoughlin wrote:

> > 	http://bugzilla.gnome.org/showattachment.cgi?attach_id=14946
> > 
> > 	It may be beneficial to allow the arrow/label to be below the child,
> > but I'm not so sure.
> > 
> > 	See below for a proposed API (different from whats in the bug). Some
> > things worth considering:
> > 
> > 	* The simplest constructor is without a label, I'm not sure if that is
> > of any use. Probably should have to at least specify a label.
> 
> Yeah, matching gtk_frame_new (const gchar *label) [and allowing NULL]
> would seem best to me.

	Okay, done. I've also added new_with_mnemonic and the then neccessary
set/get_use_underline methods.

> > 	* Should we put "spacing" in the constructors like gtk*box?
> 
> IMO, no. The reason I don't like this is that while the label is
> obvious when reading code, a number 
> 
>  gtk_expander_new ("Hello", 5)
> 
> is not obviously meaningful.

	Good point.

> > 	* Matthias suggests that it may be worthwhile to allow arbitrary
> > widgets to replace the label.
> 
> Definitely. I'd probably copy GtkFrame for the API.

	Right, I've blatantly copied GtkFrame.

> > 	* I considered adding gtk_expander_expand/collapse but I don't think it
> > adds much.
> 
> Sounds just confusing to me to have both that and 
> gtk_expander_set/get_expanded.

> > 	* I haven't exposed anything in the instance struct. It may be
> > worthwhile to expose the label widget there.
> 
> As long as there is a getter for the label/label_widget, I don't
> think it's necesary.
> 
> Looking at GtkFrame, we made:
> 
>  gtk_frame_set_label (frame, "Hello");
>  gtk_frame_get_label_widget (frame);
> 
> Return the label widget not NULL.  (Hmm, bug 108305.)
> 
> I'm not sure this is actually best ... the properties behave the same
> way, which will force a GUI builder to special case the 'label_widget'
> property. But since GtkFrame works that way, it's probably best if
> GtkExpander works the same.
> 
> > 	Overall though, I think the API should be a simple subset of
> > GtkToggleButton/GtkButton.
> 
> GtkFrame seems more like the closer comparison, as bin-plus-extra-child.
> 
> > struct _GtkExpanderClass
> > {
> >   GtkBinClass  parent_class;
> > 
> >   void       (*toggled) (GtkExpander *expander);
> 
> I'd say people can just connect to notify::expanded. The duplication
> shouldn't be necessary, and it's not like observing the state of
> the widget is going to be common anyways.

	I've removed the "toggled" signal, but one point of confusion that may
remain now is that I've added an "activate" signal for widget
activation. I think people may go looking for a "toggled" signal and
assume "activate" is what they're looking (indeed, it may work fine in
some cases). Maybe I worry too much, though.

	I've appended the API as it stands currently and added the new
implementation to #60553. If you're okay with the API it'd be good if
you had a chance to review the implementation .... :-)

Good Luck,
Mark.

P.S. - yeah, I know GTK_EXPANDER_GET_PRIVATE shouldn't be exposed - I
just spotted that.

#define GTK_TYPE_EXPANDER            (gtk_expander_get_type ())
#define GTK_EXPANDER(obj)            (G_TYPE_CHECK_INSTANCE_CAST ((obj), GTK_TYPE_EXPANDER, GtkExpander))
#define GTK_EXPANDER_CLASS(klass)    (G_TYPE_CHECK_CLASS_CAST ((klass), GTK_TYPE_EXPANDER, GtkExpanderClass))
#define GTK_IS_EXPANDER(obj)         (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GTK_TYPE_EXPANDER))
#define GTK_IS_EXPANDER_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GTK_TYPE_EXPANDER))
#define GTK_EXPANDER_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), GTK_TYPE_EXPANDER, GtkExpanderClass))
#define GTK_EXPANDER_GET_PRIVATE(o)  (G_TYPE_INSTANCE_GET_PRIVATE ((o), GTK_TYPE_EXPANDER, GtkExpanderPrivate))

typedef struct _GtkExpander      GtkExpander;
typedef struct _GtkExpanderClass GtkExpanderClass;

struct _GtkExpander
{
  GtkBin bin;
};

struct _GtkExpanderClass
{
  GtkBinClass parent_class;

  void (* activate) (GtkExpander *expander);
};

GType                 gtk_expander_get_type          (void);

GtkWidget            *gtk_expander_new               (const gchar *label);
GtkWidget            *gtk_expander_new_with_mnemonic (const gchar *label);

void                  gtk_expander_set_expanded      (GtkExpander *expander,
						      gboolean     expanded);
gboolean              gtk_expander_get_expanded      (GtkExpander *expander);

/* Spacing between the expander/label and the child */
void                  gtk_expander_set_spacing       (GtkExpander *expander,
						      gint         spacing);
gint                  gtk_expander_get_spacing       (GtkExpander *expander);

void                  gtk_expander_set_label         (GtkExpander *expander,
						      const gchar *label);
G_CONST_RETURN gchar *gtk_expander_get_label         (GtkExpander *expander);

void                  gtk_expander_set_use_underline (GtkExpander *expander,
						      gboolean     use_underline);
gboolean              gtk_expander_get_use_underline (GtkExpander *expander);

void                  gtk_expander_set_label_widget  (GtkExpander *expander,
						      GtkWidget   *label_widget);
GtkWidget            *gtk_expander_get_label_widget  (GtkExpander *expander);