membraneframework-labs / membrane_videoroom

Apache License 2.0
42 stars 8 forks source link

Add React and basic video streaming features #59

Closed kamil-stasiak closed 1 year ago

kamil-stasiak commented 2 years ago

What works:

Not yet implemented:

Needs work:

css grid image

Features added only for development

screen-sharing icon on peer tile image
random emoji icon as a user metadata image
camera streaming indicator image
kamil-stasiak commented 2 years ago
  • nice use of hooks 👍

Thank you 😊

  • do you think it would be wise to have membrane hooks as a library for others to use? we have membrane for React Native (it has even a similar api to your hooks), wouldn't it be a good idea to have membrane for React as well if we're writing hooks etc anyway?

I think it would be nice to extract hooks as a library. We have to have a look also at 'membrne live' project.

  • are you going to write the whole frontend in one PR? this would be a nightmare to review 😅 and recently we had a talk about dividing up tasks

This is the first step. I will add new commits as PR to this PR. Then I will merge everything to the main branch.

  • is it normal in web dev that you have 200+ character long lines with styles o.o? sometimes partially copied? seems not readable to me (I code in React Native and I've never used tailwind before, just normal styles and styled-components)

It's tailwind way. It's strange but it works.

  • why is camera off by default ;_;

Only for development. It's easier to test. W will turn camera on by default when I implement permission manager.

This is nice! I forgot about It!

Thank you!

bblaszkow06 commented 2 years ago

@kamil-stasiak please rename the branch to the proper prefix (Rewrite is MV-87, not MV-82)

kamil-stasiak commented 2 years ago

@kamil-stasiak please rename the branch to the proper prefix (Rewrite is MV-87, not MV-82)

Final merge will be from branch with right name.