Closed
Bug 1131688
Opened 10 years ago
Closed 10 years ago
In the standalone view the conversation window we're currently overlaying the toolbar on top of the remote video
Categories
(Hello (Loop) :: Client, defect, P1)
Hello (Loop)
Client
Tracking
(firefox38 fixed)
People
(Reporter: standard8, Assigned: standard8)
References
Details
Attachments
(1 file)
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.
Assignee | ||
Comment 1•10 years ago
|
||
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.
Attachment #8562272 -
Flags: review?(dmose)
Comment 2•10 years ago
|
||
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?
Assignee | ||
Comment 3•10 years ago
|
||
(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•10 years ago
|
||
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.
Attachment #8562272 -
Flags: review?(dmose) → review+
Assignee | ||
Comment 5•10 years ago
|
||
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.
Assignee | ||
Comment 6•10 years ago
|
||
Target Milestone: --- → mozilla38
Comment 7•10 years ago
|
||
Updated•10 years ago
|
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•