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

Grid component #170

Merged
merged 16 commits into from
Jan 30, 2024
Merged

Grid component #170

merged 16 commits into from
Jan 30, 2024

Conversation

thyal
Copy link
Member

@thyal thyal commented Dec 14, 2023

A (simplified) port of Whereby's grid logic. Adds a new component <VideoGrid />.

For now, the only grid ported over is the video stage grid. Presentation grid and subgrid is not included yet, but might be at a later point.

The PR is quite big, but most of the code is just ported over, and doesn't need an in-depth review
(The files in the /helpers folder).

The API of the component can be changed - for now it's quite simple (but still flexible).
The idea is that there's two main ways of using this: with or without custom rendering of the video cells. For now, preserving aspect ratio is just supported if the cells are not custom rendered.

Things that are missing:

  • Aspect ratio is not reported on first render - only after a change has happened in the room
  • More customize options (f.ex rounded corners)
  • Resolution reporting to the SFU
  • Fix onResize prop cast to any

@thyal thyal changed the base branch from main to development December 14, 2023 14:47
@thyal thyal force-pushed the thomas/grid-layout branch from 69d6eb9 to 1216803 Compare January 15, 2024 10:56
@thyal thyal changed the base branch from development to main January 15, 2024 10:56
@thyal thyal force-pushed the thomas/grid-layout branch 2 times, most recently from 468fdc9 to 3729f9d Compare January 26, 2024 14:12
@thyal thyal changed the base branch from main to development January 26, 2024 14:22
@thyal thyal changed the title grid layout Grid component Jan 26, 2024
@thyal thyal marked this pull request as ready for review January 26, 2024 14:28
@thyal thyal requested a review from a team January 26, 2024 14:29
Copy link
Collaborator

@havardholvik havardholvik left a comment

Choose a reason for hiding this comment

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

Works as expected. Should be good enough for a beta release 👍

if (onSetAspectRatio) {
const h = videoEl.current.videoHeight;
const w = videoEl.current.videoWidth;
if (w && h && w + h > 20) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe extract this or document it a bit? I see this is used below as well

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah good point. I'll extract it out to a function

Copy link
Member Author

Choose a reason for hiding this comment

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

I just removed this for now.

src/lib/react/Grid/index.tsx Show resolved Hide resolved
@thyal thyal force-pushed the thomas/grid-layout branch from 62c80fe to d1975b1 Compare January 30, 2024 08:41
@thyal thyal merged commit 1667b52 into development Jan 30, 2024
2 checks passed
@thyal thyal deleted the thomas/grid-layout branch January 30, 2024 08:55
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