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

Replace device dropdowns with radio buttons #2803

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

robintown
Copy link
Member

This is closer to what the designs actually want device settings to look like, and it avoids the visual glitch in which the dropdown would render underneath the slider.

Screenshot from 2024-11-19 17-24-11

@toger5
Copy link
Contributor

toger5 commented Nov 19, 2024

Do you think this is the right place to also work on the default device bahaviour?
Iirc on chrome there is a virutal default device that is always in sync with the system default and on Firefox the order of the devices change in a ways o the first one is always the default device.
I could imagine that a checkbox Default could be added and the options will only become selectable once the checkbox is disabled?

Comment on lines +99 to +105
{!isFirefox() && (
<DeviceSelection
devices={devices.audioOutput}
caption={t("settings.speaker_device_selection_label")}
/>
)}
<div className={styles.volumeSlider}>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might not be needed. (anymore)
This is what i get with the WebRtc test page https://webrtc.github.io/samples/src/content/devices/input-output/

It seems like both platforms support input and output devices.

Chrome

Screenshot from 2024-11-20 10-42-25
Screenshot from 2024-11-20 10-42-21

Firefox

Screenshot from 2024-11-20 10-53-51
Screenshot from 2024-11-20 10-53-49

Copy link
Contributor

Choose a reason for hiding this comment

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

But I am not sure how much of the output selection even works. On linux It seems like I mostly just use the system level output/input.

Copy link
Member Author

Choose a reason for hiding this comment

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

Past me says that we were waiting for Firefox 123 in order to turn it back on: #2022 (comment) so I'll do so in a follow-up PR.

This is closer to what the designs actually want device settings to look like, and it avoids the visual glitch in which the dropdown would render underneath the slider.
@robintown
Copy link
Member Author

Do you think this is the right place to also work on the default device bahaviour? Iirc on chrome there is a virutal default device that is always in sync with the system default and on Firefox the order of the devices change in a ways o the first one is always the default device. I could imagine that a checkbox Default could be added and the options will only become selectable once the checkbox is disabled?

I'd like to avoid adding a switch that blocks you from changing your device until it's turned off. But perhaps we could normalize the behavior between Firefox and Chrome by adding a virtual 'Default' device in Firefox? If that sounds appropriate to you I could work on that in a later PR.

@toger5
Copy link
Contributor

toger5 commented Nov 21, 2024

This works really great! Code looks good and I tested it a bit.

@robintown robintown merged commit 8de9687 into element-hq:livekit Nov 21, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants