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.

Sign up for GitHub

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

Jump to bottom

feat(media): call ui updates #4553

Merged
merged 14 commits into from
Sep 7, 2022
Merged

feat(media): call ui updates #4553

merged 14 commits into from
Sep 7, 2022

Conversation

jasonwoodland
Copy link
Contributor

What this PR does 📖

  • Improve volume slider
  • Improve drag bar
  • Calculate optimal MediaUser dimensions
  • Refactor calling UI
  • Update talking indicator

Which issue(s) this PR fixes 🔨

Special notes for reviewers 🗒️

Additional comments 🎤

@jasonwoodland jasonwoodland force-pushed the 4526-calling-video-ui-updates branch from c0ccf76 to 22be181 Compare August 31, 2022 11:58
@github-actions github-actions bot added the Missing Dev Review A Dev and a Dev Approver need to review the PR, then mark as Ready for QA label Aug 31, 2022
@WanderingHogan
Copy link
Contributor

the only issue I saw was there was three video windows on a direct call

@jasonwoodland
Copy link
Contributor Author

the only issue I saw was there was three video windows on a direct call

@WanderingHogan got a screenshot? probably the corrupt video state problem I mentioned before, the video/screen stream muted state is not being initialised correctly.

@github-actions github-actions bot added the missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa label Sep 1, 2022
@stavares843
Copy link
Member

stavares843 commented Sep 2, 2022 •
edited
Loading

the only issue I saw was there was three video windows on a direct call

this is also on dev, phil noticed this as well yesterday

Screen_Recording_2022-08-31_at_11.16.09_AM.1.mov

the flow was when using screen share would do a extra window as phil mentioned on dev

@WanderingHogan
Copy link
Contributor

the rebase on this one isn't so bad, conflicts are pretty minor, but it looks like a lot of other things changed a little bit so we will need to test it pretty well

@jasonwoodland
Copy link
Contributor Author

the only issue I saw was there was three video windows on a direct call

this is also on dev, phil noticed this as well yesterday

Screen_Recording_2022-08-31_at_11.16.09_AM.1.mov
the flow was when using screen share would do a extra window as phil mentioned on dev

This was due to invalid muted stream state. Will be fixed by another PR

@jasonwoodland jasonwoodland force-pushed the 4526-calling-video-ui-updates branch from c039b3b to 67ccca6 Compare September 7, 2022 00:28
@jasonwoodland jasonwoodland removed the missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa label Sep 7, 2022
josephmcg
josephmcg suggested changes Sep 7, 2022
View reviewed changes
Copy link
Contributor

@josephmcg josephmcg left a comment

Choose a reason for hiding this comment

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

the volume sliders on the settings page use the same component as the call (for some reason lol). may need some refactoring
image

also get this error on receiver side
image

@josephmcg josephmcg added Changes Requested Changes have been requested by Dev or QA, once the changes have been made, remove this label. and removed Missing Dev Review A Dev and a Dev Approver need to review the PR, then mark as Ready for QA labels Sep 7, 2022
@jasonwoodland jasonwoodland added Missing Dev Review A Dev and a Dev Approver need to review the PR, then mark as Ready for QA and removed Changes Requested Changes have been requested by Dev or QA, once the changes have been made, remove this label. labels Sep 7, 2022
@josephmcg josephmcg self-requested a review September 7, 2022 04:53
@josephmcg
Copy link
Contributor

still getting the same error from before. Looks like the refs inside of the mounted logic are coming up undefined (for one side only)

@josephmcg josephmcg merged commit a2dc8f9 into dev Sep 7, 2022
@josephmcg josephmcg deleted the 4526-calling-video-ui-updates branch September 7, 2022 09:07
@github-actions github-actions bot removed the Missing Dev Review A Dev and a Dev Approver need to review the PR, then mark as Ready for QA label Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stavares843 stavares843 stavares843 left review comments

@WanderingHogan WanderingHogan WanderingHogan left review comments

@josephmcg josephmcg josephmcg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jasonwoodland @WanderingHogan @stavares843 @josephmcg