Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the layouts on small mobile calls #2514

Merged
merged 4 commits into from
Aug 6, 2024

Conversation

robintown
Copy link
Member

Due to an oversight of mine, 2440037 actually removed the ability to see the one-on-one layout on mobile. This restores mobile one-on-one calls to working order and also avoids showing the spotlight tile unless there are more than a few participants.

Based on #2512 and #2511

@toger5
Copy link
Contributor

toger5 commented Jul 26, 2024

This is super nice. This was my main personal issue with the new layout.
Are those also the current designs?
I have in mind, that I saw a design that has a borderless video for 1:1 calls?
If that is true, is this the right place to update it?

Edit: borderless is only in landscape mode. Here we also have a feed orientation issue (the feed is still in portrait if the phone is in landscape mode)

I am wondering if this is missing though since there are no designs for portrait mobile with less than a handful if participants. 1 and 2 participants might make sense to show as borderless however? Like the lobby designs?

@fkwp
Copy link
Contributor

fkwp commented Jul 26, 2024

nice :-) I just observed in landscape mode I have two times self in 1:1 rather remote and self. Does that depend on the other PRs floating around?

@toger5
Copy link
Contributor

toger5 commented Jul 30, 2024

Blocked because of merge conflicts.

Due to an oversight of mine, 2440037 actually removed the ability to see the one-on-one layout on mobile. This restores mobile one-on-one calls to working order and also avoids showing the spotlight tile unless there are more than a few participants.
@robintown
Copy link
Member Author

@fkwp Fixed that in #2528

I am wondering if this is missing though since there are no designs for portrait mobile with less than a handful if participants. 1 and 2 participants might make sense to show as borderless however? Like the lobby designs?

@toger5 Updated the PR with this suggestion and rebased on livekit ✔️

Copy link
Contributor

@toger5 toger5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good.

I have a couple of comments about the behaviour:

I think we should not base the switch between borderless and with-border based on the aspect ratio.
Portrait on a small screen with only two participants should also be borderless. Otherwise we get some odd window resizing behaviour where it can switch between borderless and with-boarder rapidly.

I think sth like
"aspect videomedia track/ aspect screen about the same" and "screen area < threshold" -> no border
would be simpler?

I also think lobby and inCallView should use the same logic.

I think if there is only one person in the call we should also only show one video tile. (currently we mirror our own tile into the main tile slot)

@robintown robintown requested a review from toger5 August 6, 2024 13:40
Copy link
Contributor

@toger5 toger5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed in a meeting.

  • two tiles comment gets addressed in a different PR
  • It turns out that most of my feedback is addressable with some value tweaking

@robintown robintown enabled auto-merge August 6, 2024 14:09
@robintown robintown merged commit ab42fe9 into element-hq:livekit Aug 6, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants