Re: Table menu patch for GtkMenu

[Owen Taylor <otaylor redhat com>]

On Thu, 2003-07-24 at 00:09, Kristian Rietveld wrote:
> On Tue, 2003-07-15 at 16:34, Owen Taylor wrote:
> > The API is basically fine, though I'd like to see some explanation
> > of what "gtk_menu_occupied" is for, but I have various large-scale
> 
> gtk_menu_occupied() was for checking if a given place is already
> occupied by a menu item. But now I moved to child properties, as you
> suggested, this function is no longer needed. So now it only appears as
> a GtkComboBox internal.

Makes sense to me... it really doesn't seem all that generally useful,
so if you can just implement it where it you need it, that seems
best.

> > thoughts about the implementation:
> 
> I think I addressed all of your comments in the new patch, which I've
> attached. Some notes:
> 
> - in the size requisition/allocation algorithm, the width is
> homogeneous, but the height is not. Why is this? For example, think of a
> menu with usual text items and a separator item. Then the separator item
> *has* to have another height then the text items in order to look nice.

Maybe *neither* should be homogeneous - the mix seems a bit confusing.
What's the inspiration for having the columns homogenous?

> - Now we have child properties, we can basically drop the
> gtk_menu_attach() function. In that case, we need to make GtkMenu to
> connect to each notify signal of all of the attach properties. Do we
> want to do this? Isn't this too much overhead? It doesn't sound like a
> good idea to me.
> 
> I hope this new patch comes closer to what you are expecting.

As Matthias said, you seem to have used properties, not child properties
here; so, well, it's not quit what I was, expecting, no :-)

Child properties are implemented in the parent container, not in
the child, so the parent doesn't need to wait for notification.

I don't think we want to drop gtk_menu_attach() - we typically expose
a straightforward C API along with properties ... the only exceptions
to that are GtkCellRenderer and GtkTextTag.

One random notes about somethins noticed when browing through
the patch (I didn't go through it line-by-line because of the
property/child-property issue.)

 * When gtk_menu_shell_insert() are called on a non-trivial table
   menu, the result is complete mangling. I think an appropriate
   intepretation would be to:

   - Attach after the last row for postiion < 0
   - Otherwise move any items where the top is >= position
     down one row, and attach the item with left_attach=0
     right_attach=1, top_attach = position,
     bottom_attach = position+1.

     (Or maybe right_attach = n_cols+1?)

Regards,
						Owen