- flex-nowrap on grid rows prevents subpixel rounding from wrapping the
second tile onto a new row (caused 'half off screen' on desktop)
- flex flex-col on mobile wrapper gives the grid container a constrained
height so it renders at narrow viewport widths
Complete rewrite of the tile grid sizing algorithm addressing all
review feedback from @matt across both reviews.
Review #1 (Jitsi-style approach):
- Tiles flex between 3:4 (portrait) and arbitrarily wide (no upper
clamp on aspect ratio) to avoid overflow.
- Mixed row sizes: when the last row is partial, those tiles are
wider (16:9 'match heights') while full rows use square-ish
tiles, creating a cohesive Jitsi-style look.
- Example: 3 participants = 2 square-ish tiles on top + 1 centered
16:9 tile on bottom.
Review #2 (n=2 half-off-screen bug + minimum size policy):
- FIX: No upper-bound clamping on aspect ratio - tiles can be wider
than 16:9 when the container is very wide. At n=2 on a wide desktop,
tiles are 16:9 (889x500) with side whitespace instead of overflowing.
- MIN_TILE_HEIGHT=120: only allows overflow (scroll) when all tiles
fall below this threshold. At n=2 tiles are always fully visible.
- For n=6-12 on a portrait phone, tiles shrink to fit within aspect
bounds with no scroll.
Inline code review feedback:
- Full words everywhere: containerWidth, columnCount, tileWidth,
tileHeight, aspectRatio, etc.
- No 'key' naming: replaced with LayoutScore struct with named fields
- Type renamed from UniformTileGrid to AdaptiveTileGrid + TileRow
- No 'compareKeys' - replaced with compareScores on named fields
Scoring: overflow penalty -> min-height penalty -> tile area ->
whitespace -> aspect deviation.
CSS grid + mx-auto centers the whole block but not orphan tiles within
their row (e.g. 5 tiles @1685x1342, the 2-tile row was left-aligned).
Switched to display:flex; flex-wrap:wrap; justify-content:center with
explicit width/height on each tile wrapper. This naturally centers every
row, including orphan rows with fewer tiles.
The computeUniformGrid algorithm is unchanged — still uniform px tile
sizes with aspect flexing in [3:4..16:9].
Bug 1 — chicken-and-egg gating: '{:else if useMultiGrid && tileLayout}'
prevented the measuring container from ever mounting, because tileLayout
is undefined while gridWidth/gridHeight are 0, and the bind:clientWidth/
clientHeight are on the same div. Fix: always mount the measuring
container when useMultiGrid, gate only the rows with {#if tileLayout}.
Bug 2 — collapsed tile height: 'default' tile class lost aspect-video but
gained no replacement for intrinsic height. All children are absolutely
positioned (VideoCallTile 'absolute inset-0', profile fallback, badges,
label, pin button), so the div collapses to 0 height. Fix: add 'h-full'
to the default tile class so it fills the aspect-ratio wrapper.
Replaces the uniform grid-sizing helper (computeGridSize -> rows/cols)
with a Jitsi-inspired mixed-size layout algorithm (computeTileLayout).
Key changes:
- Tiles flex between 9:16 and 16:9 aspect ratio per row, matched to
container dimensions
- Rows can have different tile counts (e.g. [2,1] for 3 participants,
[3,2] for 5, [4,3] for 7)
- Last row with fewer tiles gets a wider (more landscape) aspect ratio
while earlier rows stay more square-ish
- CSS grid replaced with flex rows; each tile's aspect-ratio set via
inline style (no more hardcoded aspect-video)
- Container dimensions bound via bind:clientWidth/clientHeight
- Dead {:else} branch (always-true useMultiGrid) removed
- Overflow penalty, whitespace penalty, and balance penalty ensure
reasonable layouts at all tile counts
Per @matt's PR review feedback: read jitsi-meet source
(react/features/video-layout/functions.web.ts,
react/features/filmstrip/functions.web.ts) for the tile sizing
approach.
Add computeGridSize helper in call.ts that computes optimal row/column
counts for a video tile grid given tile count and container dimensions,
maximizing tile size while maintaining ~16:9 cells.
Apply via computed grid-template-columns/rows on the multi-grid path
(no-spotlight) in VideoCallContent.svelte, replacing the fixed
grid-cols-1 / grid-cols-1 sm:grid-cols-2 heuristic. The multi-grid now
handles all tile counts from 1 to 12+ smoothly.
- computeGridSize(tileCount, containerWidth, containerHeight) -> GridSize
- Container dimensions bound via clientWidth/clientHeight
- useMultiGrid: dropped the > 2 check, now !useSpotlightLayout
- gridStyle: computed grid-template-columns/rows string from gridSize
- Scope: multi-grid only, no spotlight/strip/breakpoint changes
#135
This PR adds basic video functionality to our voice rooms. Again I followed the Discord UX for inspiration, so all video calls start as voice-only calls that gracefully upgrade (and downgrade) when someone turns on a video or starts screen sharing.
When a video feed is detected the Room page will change to display a grid of feeds. The grid logic is very basic, that's definitely an area to improve in the future. You can open the chat part of the room with a new button on the VoiceWidget - on the desktop layout this creates a split view with video on the left and chat on the right, but on mobile it switches to chat fullscreen. I also added a little pin icon you can use to focus on a single video feed (useful for screen sharing). There is a lot of tailwind I don't understand here, but it seems to work well enough.
I moved voice.ts into a new `call` folder and moved some of its stores into `call/stores.ts` which allowed me to keep most of the video logic in `call/video.ts`. It's not a perfect encapsulation as voice.ts does subscribe to some of the hooks for the livekit calls and passes some of the signals onto `video.ts`. This could probably be broken up better but for this PR I'd rather not focus on making it perfect if that's ok. Partly for the sake of time but also because I envision another PR that renames/reorganizes things and I think a larger UX evaluation is necessary and should include real user feedback. I'm not confident tha""t the Voice Room concept as a whole will stick going forward. Maybe all rooms in a livekit enabled server should be able to host a call (like a slack huddle), maybe users want to be able to schedule calls as events, or even have them start with an ad-hoc set of participants completely outside of a NIP-29 group, etc.
Co-authored-by: mplorentz <mplorentz@noreply.gitea.coracle.social>
Reviewed-on: https://gitea.coracle.social/coracle/flotilla/pulls/135
Co-authored-by: Matt Lorentz <mplorentz@noreply.coracle.social>
Co-committed-by: Matt Lorentz <mplorentz@noreply.coracle.social>
