Improve the look of voice calls and group calls on mobile#4067
Improve the look of voice calls and group calls on mobile#4067robintown wants to merge 7 commits into
Conversation
robintown
added
PR-Breaking-Change
| }); | ||
| }); | ||
|
|
||
| describe("videoEnabled", () => { |
There was a problem hiding this comment.
Removed these tests because they rested on the assumption that display: none was set directly in the video's style attribute rather than in a style sheet.
robintown
force-pushed
the
mobile-gradient
branch
from
1653a8c to
7b03f07
Compare
robintown
force-pushed
the
mobile-gradient
branch
from
7b03f07 to
b3a8807
Compare
robintown
force-pushed
the
mobile-gradient
branch
from
b3a8807 to
701913b
Compare
By giving spotlight tiles a transparent background in certain layouts.
To indicate when the user is speaking.
By visually separating it from other page content with an additional gradient background only when necessary.
robintown
force-pushed
the
mobile-gradient
branch
from
701913b to
900d0ab
Compare
| this.maximised$.next(value); | ||
| } | ||
|
|
||
| private readonly bgStyle$: BehaviorSubject<"solid" | "transparent">; |
There was a problem hiding this comment.
Nit: Maybe worth extracting a type given that "solid" | "transparent" occurs in multiple places?
| ref={ref} | ||
| data-testid="videoTile" | ||
| data-video-fit={videoFit} | ||
| data-bg-style={bgStyle} |
There was a problem hiding this comment.
I can't help but notice the imparity between bgStyle / data-bg-style and background / data-background (used for the page background) which both set the style of the background. This doesn't really matter much; so you may do with this comment whatever you want. 🙈😅
| /> | ||
| <div | ||
| class="footer" | ||
| class="footer footer" |
There was a problem hiding this comment.
This struck me as odd at first but looks like these are in fact two different classes that just end up using the same string in tests due to our classNameStrategy setting. I guess classNameStrategy == stable would call this out a little nice but I assume there's no real downsides with the duplicate class names in just tests either?
Johennes
left a comment
Johennes
left a comment
There was a problem hiding this comment.
This looks good to me code-wise. Thanks a ton for breaking it up into commits. That really simplified the review.
I only tested Android. Is this how it is supposed to look without further changes?
| Before | After |
|---|---|
 |
 |
2 participants
This improves the look of calls on mobile by adding a new gradient background and adjusting various visual elements (footer and spotlight tiles) to let the background "shine through" when appropriate.
It also applies a new "sound wave" design to the other participant's avatar when they are speaking. If desired I can split that change out.
You can review this commit-by-commit.
I've marked this as a breaking change because it now applies a gradient background by default even in widget mode, which consumers of the embedded package will need to adapt to. (Element Web will want to disable the gradient, and Element X mobile clients will want to make it look best by forcing a dark theme.)
Closes https://github.com/element-hq/voip-internal/issues/526