Created attachment 193403 [details] [review]
CallWindow: make the toolbar float inside the window

Created attachment 193404 [details] [review]
CallWindow: hide the toolbar after 3s of inactivity

Created attachment 193405 [details] [review]
CallWindow: show the toolbar when on fullscreen

And also hide it after 3s of inactivity.

Created attachment 193406 [details] [review]
Add a GtkClutterActor subclass that clips the corners

Created attachment 193407 [details] [review]
Make the floating toolbar have rounded corners

Created attachment 193408 [details] [review]
Use a BindConstraint instead of a SnapConstraint

It seems less hacky even though we need to change the constraint
when the allocation changes.

Created attachment 193414 [details] [review]
Move the video preview up

So it's not on top of the floating toolbar.

Review of attachment 193403 [details] [review]:

::: src/empathy-call-window.c
@@ +548,3 @@
   priv->video_preview = clutter_box_new (layout);
   clutter_actor_set_size (priv->video_preview,
+      SELF_VIDEO_SECTION_WIDTH + 10, SELF_VIDEO_SECTION_HEIGTH + 10 + 20 + 36);

What is 10+20+30 ? Why is it like that instead of +60? Can we use #defines here.

Review of attachment 193404 [details] [review]:

::: src/empathy-call-window.c
@@ +797,3 @@
+    {
+      g_source_remove (self->priv->inactivity_src);
+static gboolean

Unrequired assignment.

Review of attachment 193408 [details] [review]:

::: src/empathy-call-window.c
@@ +852,3 @@
+
+  clutter_bind_constraint_set_offset (constraint,
+  ClutterActorBox allocation;

Unrequired brackets.

@@ +996,3 @@
 
+  constraint = clutter_bind_constraint_new (priv->video_box,
+      CLUTTER_BIND_Y, 70);

What is 70?

Review of attachment 193414 [details] [review]:

::: src/empathy-call-window.c
@@ +555,3 @@
+      SELF_VIDEO_SECTION_WIDTH + 10,
+      SELF_VIDEO_SECTION_HEIGTH + 10 +
+      FLOATING_TOOLBAR_HEIGHT + FLOATING_TOOLBAR_SPACING);

What is 10?

(In reply to comment #8)
> Review of attachment 193403 [details] [review]:
> 
> ::: src/empathy-call-window.c
> @@ +548,3 @@
>    priv->video_preview = clutter_box_new (layout);
>    clutter_actor_set_size (priv->video_preview,
> +      SELF_VIDEO_SECTION_WIDTH + 10, SELF_VIDEO_SECTION_HEIGTH + 10 + 20 +
> 36);
> 
> What is 10+20+30 ? Why is it like that instead of +60? Can we use #defines
> here.

Another commit reverted that change, so I've squashed that commit.

(In reply to comment #9)
> Review of attachment 193404 [details] [review]:
> 
> ::: src/empathy-call-window.c
> @@ +797,3 @@
> +    {
> +      g_source_remove (self->priv->inactivity_src);
> +static gboolean
> 
> Unrequired assignment.

Removed.

(In reply to comment #10)
> Review of attachment 193408 [details] [review]:
> 
> ::: src/empathy-call-window.c
> @@ +852,3 @@
> +
> +  clutter_bind_constraint_set_offset (constraint,
> +  ClutterActorBox allocation;
> 
> Unrequired brackets.

Removed.

> @@ +996,3 @@
> 
> +  constraint = clutter_bind_constraint_new (priv->video_box,
> +      CLUTTER_BIND_Y, 70);
> 
> What is 70?

Just a random number to set an initial position for the toolbar, it's then updated to the correct position in empathy_call_window_video_box_allocation_changed_cb(). So I've changed it to 0 as 70 isn't really necessary.

(In reply to comment #11)
> Review of attachment 193414 [details] [review]:
> 
> ::: src/empathy-call-window.c
> @@ +555,3 @@
> +      SELF_VIDEO_SECTION_WIDTH + 10,
> +      SELF_VIDEO_SECTION_HEIGTH + 10 +
> +      FLOATING_TOOLBAR_HEIGHT + FLOATING_TOOLBAR_SPACING);
> 
> What is 10?

The preview margin (i.e. the space between the preview and the avatar/remote video borders. I've changed that to a define in a new commit (as that was already there).

I have also added a commit to fix a focus bug that I've just found.

And another commit to fix another warning.

++

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.