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

feat(MediaSettings) - Adjust the preview background update to only change in video when confirmed #10188

Merged

Conversation

DorraJaouad
Copy link
Contributor

☑️ Resolves

🚧 Tasks

  • Code review
  • Visual review

🏁 Checklist

@DorraJaouad DorraJaouad added enhancement feature: WebRTC 🚡 WebRTC connection between browsers and/or mobile clients feature: frontend 🖌️ "Web UI" client feature: call 📹 Voice and video calls labels Aug 11, 2023
@DorraJaouad DorraJaouad added this to the 💙 Next Major (28) milestone Aug 11, 2023
@DorraJaouad DorraJaouad self-assigned this Aug 11, 2023
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Seems to work, nothing blocking - just remarks about wording / naming

src/components/MediaSettings/MediaSettings.vue Outdated Show resolved Hide resolved
src/components/MediaSettings/MediaSettings.vue Outdated Show resolved Hide resolved
src/components/MediaSettings/MediaSettings.vue Outdated Show resolved Hide resolved
src/components/MediaSettings/MediaSettings.vue Outdated Show resolved Hide resolved
Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

Hi @DorraJaouad & @Antreesy :)
I think "Apply settings" works well for every setting there, and we should use it it in the devices page too. By that I mean that both camera and microphone device settings shouldn't immediately affect the streams in the call but only the preview videos and mic level.
Changes should occur only when "apply settings" button is clicked.
What do you think?

@Antreesy
Copy link
Contributor

Antreesy commented Aug 14, 2023

Changes should occur only when "apply settings" button is clicked. What do you think?

Should be possible to split this logic to preview and apllied device, same as for background, so I'd like to see it
UPD: or maybe it requires some modifications to be done at MediaDevicesManager, so I'd like to not mess with the working code at this point...

…firmed

Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad DorraJaouad force-pushed the feat/9379/preview_background_without_video_stream_update branch from 83732ca to febde91 Compare August 15, 2023 12:10
@DorraJaouad DorraJaouad requested a review from Antreesy August 15, 2023 12:16
Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

🎉

@Antreesy Antreesy enabled auto-merge August 18, 2023 11:50
@Antreesy
Copy link
Contributor

/backport to stable27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feature: call 📹 Voice and video calls feature: frontend 🖌️ "Web UI" client feature: WebRTC 🚡 WebRTC connection between browsers and/or mobile clients
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preview video backgrounds when in a call
3 participants