Created attachment 160569 [details] [review]
calendar: Fix day headers being elipizied

Currently some headers like Mo, Tue, Wed, ... might end up elipizied,
which should never happen because showing "..." in the UI instead of Fri,
isn't the intended behaviour.

Fix this by setting the x_fill property to true.

Created attachment 160570 [details] [review]
calendar: Fix day headers being elipizied

Currently some headers like Mo, Tue, Wed, ... might end up elipizied,
which should never happen because showing "..." in the UI instead of Fri,
isn't the intended behaviour.

Fix this by setting the x_fill property to true.

Also use the calendar-day style_class for headers to avoid odd looking
spacing.

Comment on attachment 160570 [details] [review]
calendar: Fix day headers being elipizied

This seems to break "x_align: St.Align.END"

Created attachment 160571 [details] [review]
calendar: Fix day headers being elipizied

Currently some headers like Mo, Tue, Wed, ... might end up elipizied,
which should never happen because showing "..." in the UI instead of Fri,
isn't the intended behaviour.

Fix this by making the labels WIDTH_FOR_HEIGHT rather than HEIGHT_FOR_WIDTH.

Also use the calendar-day style_class for headers to avoid odd looking
spacing.

Created attachment 160574 [details] [review]
calendar: Fix day headers being elipizied

Fix patch to actually apply (i.e not base it on a unpushed version).

Review of attachment 160574 [details] [review]:

I think all the WIDTH_FOR_HEIGHT vs. HEIGHT_FOR_WIDTH is doing is that when set to WIDTH_FOR_HEIGHT the labels no longer ellipsize for some reason or another. But Clutter.Text doesn't have WIDTH_FOR_HEIGHT support and if it did it wouldn't make sense here - the labels aren't wrapping at all.

There is some St.Table (or possibly St.Label) bug that needs to be fixed here.

(In reply to comment #6)
> Review of attachment 160574 [details] [review]:
> 
> I think all the WIDTH_FOR_HEIGHT vs. HEIGHT_FOR_WIDTH is doing is that when set
> to WIDTH_FOR_HEIGHT the labels no longer ellipsize for some reason or another.
> But Clutter.Text doesn't have WIDTH_FOR_HEIGHT support and if it did it
> wouldn't make sense here - the labels aren't wrapping at all.
> 
> There is some St.Table (or possibly St.Label) bug that needs to be fixed here.

It is not about Clutter.Text supporting WIDTH_FOR_HEIGHT or not but St.Table is calling _st_allocate_fill() in st_table_preferred_allocate() which queries the request_mode property of the child:

-----
request = CLUTTER_REQUEST_HEIGHT_FOR_WIDTH;
g_object_get (G_OBJECT (child), "request-mode", &request, NULL);
-----

and base the allocation on that; so WIDTH_FOR_HEIGHT vs. HEIGHT_FOR_WIDTH is not really "unsupported" here.

(In reply to comment #7)
> (In reply to comment #6)
> > Review of attachment 160574 [details] [review] [details]:
> > 
> > I think all the WIDTH_FOR_HEIGHT vs. HEIGHT_FOR_WIDTH is doing is that when set
> > to WIDTH_FOR_HEIGHT the labels no longer ellipsize for some reason or another.
> > But Clutter.Text doesn't have WIDTH_FOR_HEIGHT support and if it did it
> > wouldn't make sense here - the labels aren't wrapping at all.
> > 
> > There is some St.Table (or possibly St.Label) bug that needs to be fixed here.
> 
> It is not about Clutter.Text supporting WIDTH_FOR_HEIGHT or not but St.Table is
> calling _st_allocate_fill() in st_table_preferred_allocate() which queries the
> request_mode property of the child:
> 
> -----
> request = CLUTTER_REQUEST_HEIGHT_FOR_WIDTH;
> g_object_get (G_OBJECT (child), "request-mode", &request, NULL);
> -----
>
> and base the allocation on that; so WIDTH_FOR_HEIGHT vs. HEIGHT_FOR_WIDTH is
> not really "unsupported" here.

That doesn't change anything - the change you are making to calender.js doesn't make conceptual sense and shouldn't necessary.

Created attachment 160593 [details] [review]
[StTable] Do something reasonable for width-for-height sizing

Three problems in combination here:

 - By using a horizontal boxlayout, we were invoking
   width-for-height layout on the St.Table
 - St.Table was treating the flag -1 value for for_width
   as a real width, and requested its children at junk
   with.
 - That confused some recently added code to ClutterText for
   layout caching.

Here's a fix for the middle problem. You could argue that it
should be handled generically in _st_actor_get_preferred_*
instead, but that would take away the ability of an actor
to be smart and handle both height-for-width and width-for-height
automatically.

(Do we want that? Should the user have to change the request mode
in that case?)

Created attachment 160594 [details] [review]
Use a St.Bin rather than a St.BoxLayout for calendar popup

Using a horizontal St.BoxLayout for calendar container forces
width-for-height layout on the St.Table child. Since St.Table
is naturally width-for-height, this can trigger bugs and is,
at best, a bit ineffecient. Use a St.Bin instead since we don't
need any BoxLayout features.

See http://bugzilla.openedhand.com/show_bug.cgi?id=2109 for a fix for the third problem.

Comment on attachment 160594 [details] [review]
Use a St.Bin rather than a St.BoxLayout for calendar popup

Attachment 160594 [details] pushed as 5111edb - Use a St.Bin rather than a St.BoxLayout for calendar popup

Review of attachment 160593 [details] [review]:

yeah, looks right.