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

Keep tiles in a stable order #2670

Merged
merged 11 commits into from
Nov 6, 2024
Merged

Conversation

robintown
Copy link
Member

@robintown robintown commented Oct 10, 2024

This introduces a new layer of abstraction on top of MediaViewModel: TileViewModel, which gives us a place to store data relating to tiles rather than their media, and also generally makes it easier to reason about tiles as they move about the call layout. I have created a class called TileStore to keep track of these tiles.

This allows us to swap out the media shown on a tile as the spotlight speaker changes, and avoid moving tiles around unless they really need to jump between the visible/invisible regions of the layout.

Closes #2562

This introduces a new layer of abstraction on top of MediaViewModel: TileViewModel, which gives us a place to store data relating to tiles rather than their media, and also generally makes it easier to reason about tiles as they move about the call layout. I have created a class called TileStore to keep track of these tiles.

This allows us to swap out the media shown on a tile as the spotlight speaker changes, and avoid moving tiles around unless they really need to jump between the visible/invisible regions of the layout.
Since we now assume that the spotlight and grid will be in sync (i.e. an active speaker in one will behave as an active speaker in the other), we don't want the spotlight to ever lag behind due to throttling. If this causes usability issues we should maybe look into making LiveKit's 'speaking' indicators less erratic first.
Although we try now to avoid layout shifts due to the spotlight speaker changing wherever possible, a spotlight speaker coming from off screen can still trigger one. Let's shift the layout a bit more gracefully in this case.
Copy link
Member

@hughns hughns left a comment

Choose a reason for hiding this comment

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

There is a lot going on in this PR. I've reviewed it as much as I think I reasonably can. I understand it has been reasonably tested with real usage. As such I think it is good enough to merge.

It does need the merge conflicts resolving first.

@hughns hughns merged commit d3f069e into element-hq:livekit Nov 6, 2024
6 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.

Grid view in v0.6.0 shifts people around constantly
2 participants