1200693 - Modify aspect ratio so that top and bottom black bars are not seen when waiting alone in the conversation window

Status: RESOLVED FIXED

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

Comment 1

[Romain Testard [:RT]]

We agreed that we should modify the aspect ratio of the video self video when waiting alone in the conversation (desktop client UI) so that the top and bottom black bars are not displayed.

Comment 2

[Romain Testard [:RT]]

Attachment: Capture d’écran 2015-09-01 à 18.37.37.png

Comment 3

[Mark Banner (:standard8)]

If we shorten it, then we'll need to re-check all the views, so adding to the user story.

Comment 4

[:shell escalante]

there is a fixed width when docked. height varies based on text and context.  should be way to minimize the bars though.

Comment 5

[:shell escalante]

added see also bug 1205684 - as fixing one will impact the other (or should check to see what happens)

Comment 6

[:shell escalante]

moved to Manuel and Fernando - to at least work out what is wanted and it's related to bug 1205684 

reach out to Mark/Dan as they are different parts of code - just same part of UX

Comment 7

[Manuel Casas Barrado [:mancas]]

Why we don't use object-fit: cover to solve this issue? AFAIK, it's the same we're doing with the remote video.

Comment 8

[Mark Banner (:standard8)]

(In reply to Manuel Casas Barrado [:mancas] from comment #7)
> Why we don't use object-fit: cover to solve this issue? AFAIK, it's the same
> we're doing with the remote video.

This is for privacy reasons. We don't want to display a local video stream where you can't see everything that you're sending.

For example, if you had something in the room you didn't want to show, with the cover mode, you'd think that it wasn't being captured because its not in shown in the window. However, when your friend connects, then it is suddenly shown.

There's also the case I've had where someone else comes into the room and doesn't like to be seen, so keeps off-camera and uses the local video that they can see as a guide.

Comment 9

[Manuel Casas Barrado [:mancas]]

I see your point Mark. I have a question related to the conversation window. The minimal height of the chatbox is set in |browser.css:828| as you can see in the DOM Inspector, right? However, this file is auto-generated in the build process isn't it?

Comment 10

[Mark Banner (:standard8)]

The various heights are set here:

http://hg.mozilla.org/mozilla-central/annotate/acdb22976ff8/browser/base/content/browser.css#l917

Comment 11

[Manuel Casas Barrado [:mancas]]

Attachment: Fix: Top and bottom black bars are not seen anymore

Comment 12

[Manuel Casas Barrado [:mancas]]

Comment on attachment 8667279 [details] [diff] [review]
Fix: Top and bottom black bars are not seen anymore

Hey Dan, could you take a look at the patch when you get a chance, and give me some feedback if needed?

Comment 13

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8667279 [details] [diff] [review]
Fix: Top and bottom black bars are not seen anymore

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

I'm not going to have time to look at this today, so I'm adding a couple of other potential reviewers.  If they don't get to it, I will tomorrow.

Comment 14

[Mark Banner (:standard8)]

Comment on attachment 8667279 [details] [diff] [review]
Fix: Top and bottom black bars are not seen anymore

The min & max height are interfering with the custom sizes we use for adjusting the height of the room when there's context added, or when a remote user joins the room and starts doing text chat.

I would suggest just changing the height to 255px, and do the same in .chatbar-innerbox lower down.

However, with no context and no-one in the room we also need to adjust the edit context view - on Mac, we're just starting to get scrollbars, I suspect on windows/linux it might be worse.

Comment 15

[Manuel Casas Barrado [:mancas]]

Attachment: chatboxes.png

Hey Pau, as we talked offline, I've increase the width and height of the conversation window. Now the top and bottom black bars are not shown. Could you take a quick look at the different states of the conversation window and check that everything looks good?

Comment 16

[Pau Masiá [:Pau]]

Comment on attachment 8669616 [details]
chatboxes.png

Awesome! Thanks, Manu.

Comment 17

[Manuel Casas Barrado [:mancas]]

Attachment: Fix: Top and bottom black bars are not shown

Comment 18

[Mark Banner (:standard8)]

Comment on attachment 8669628 [details] [diff] [review]
Fix: Top and bottom black bars are not shown

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

Ok, this looks fine, but please update the two extra instances that I've commented on. r=Standard8 with those fixed.

::: browser/base/content/browser.css
@@ +923,5 @@
>    width: 260px; /* CHAT_WIDTH_OPEN in socialchat.xml */
>  }
>  
>  chatbox[customSize] {
> +  width: 350px; /* CHAT_WIDTH_OPEN_ALT in socialchat.xml */

Please update the value in socialchat.xml as well.

@@ +928,4 @@
>  }
>  
>  #chat-window[customSize] {
>    min-width: 300px;

I think this should be kept in sync with the width, i.e. 350px.

Comment 19

[Manuel Casas Barrado [:mancas]]

Attachment: Fix: Top and bottom black bars are not shown

Comment 20

[Manuel Casas Barrado [:mancas]]

New patch with the minor changes that :Standard 8 said in Comment 18. Keeping the r+ since there are just a few css changes but feel free to make a quick review, if you think it's necessary

Comment 21

[Mark Banner (:standard8)]

Attachment: Modify aspect ratio so that top and bottom black bars are not seen when waiting alone in the conversation window.

It wasn't quite right. Looking back at comment 18 I could have been clearer (and the context there didn't help). CHAT_WIDTH_OPEN_ALT was the one to alter in socialchat.xml (as that's the one that Loop uses).

This is with the change applied to the ALT version rather than the other one.

Comment 22

[Pulsebot]

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

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b96e1e328e6d