1131688 - In the standalone view the conversation window we're currently overlaying the toolbar on top of the remote video

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Mark Banner (:standard8)]

Currently the conversation toolbar overlaps the remote video on the standalone UI. This means we loose the lower part of the video - this is most obvious with screen sharing.

I also want to fix this ahead of bug 1126321.

Comment 1

[Mark Banner (:standard8)]

Attachment: In the standalone view the conversation window we're currently overlaying the toolbar on top of the remote video.

This fixes the issue by changing the height of the remote media to be reduced by the height of the conversation toolbar - except when we're in-call, or if we're shrunk down small.

I didn't want to change the height of the video wrapper/media elements as that affects the local media as well.

Comment 2

[Mike de Boer [:mikedeboer]]

Comment on attachment 8562272 [details] [diff] [review]
In the standalone view the conversation window we're currently overlaying the toolbar on top of the remote video.

Review of attachment 8562272 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/loop/content/shared/css/conversation.css
@@ +32,5 @@
>  }
>  
> +.standalone .remote-stream {
> +  /* Set at maximum height, minus height of conversation toolbar */
> +  height: calc(100% - 64px);

Does this work when you resize the browser, as in doesn't the resizing code override this style rule?

I'd expect that the toolbar element would reside _outside_ of the stream, so that it'd be positioned relatively to the stream container. Wouldn't that be easier?

Comment 3

[Mark Banner (:standard8)]

(In reply to Mike de Boer [:mikedeboer] from comment #2)
> > +.standalone .remote-stream {
> > +  /* Set at maximum height, minus height of conversation toolbar */
> > +  height: calc(100% - 64px);
> 
> Does this work when you resize the browser, as in doesn't the resizing code
> override this style rule?

If you're thinking of the resizing code in updateVideoContainer to set it to 100%, then no it doesn't affect this - as that's on a different element.

> I'd expect that the toolbar element would reside _outside_ of the stream, so
> that it'd be positioned relatively to the stream container. Wouldn't that be
> easier?

I tried it, and it gave an issue with the inset stream - which would then also be shifted up 64px. In retrospect I think we might be able to adjust where the inset stream is, and force it over the toolbar - I'll have a look another look at that approach and see if its any simpler.

Comment 4

[Mike de Boer [:mikedeboer]]

Comment on attachment 8562272 [details] [diff] [review]
In the standalone view the conversation window we're currently overlaying the toolbar on top of the remote video.

Review of attachment 8562272 [details] [diff] [review]:
-----------------------------------------------------------------

Stealing review, Mark and I discussed this over Vidyo and looks like a big improvement.

Comment 5

[Mark Banner (:standard8)]

Just to clarify a couple of things for future reference - Moving the conversation toolbar in the dom tree also affects where the local video gets displayed. So we'd need to adjust its absolute position (vertically) to overlap with the toolbar.

Additionally, the media elements heights are currently set to 100%, so we'd also need to calculate the height minus the toolbar as well.

Hence, the additional work necessary doesn't seem worth it.

Comment 6

[Mark Banner (:standard8)]

https://hg.mozilla.org/integration/fx-team/rev/34652ec8485c

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/34652ec8485c