-
Notifications
You must be signed in to change notification settings - Fork 48
feat: add call controls to the screen-sharing window #381
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,27 +1,22 @@ | ||
| # Clear Warnings, Clippy & Formatting | ||
| # Clear Warnings and Clippy | ||
|
|
||
| Fix all compiler warnings, clippy lints, and formatting issues in this project. Work in this order: | ||
| Fix all compiler warnings and clippy lints in this project without changing behavior. Work in this order: | ||
|
|
||
| ## 1. Formatting | ||
| ```bash | ||
| cargo fmt --all -- --check | ||
| ``` | ||
| Fix any formatting issues reported. run `cargo fmt` directly. | ||
| ## 1. Build warnings | ||
|
|
||
| ## 2. Build Warnings | ||
| ```bash | ||
| task build_dev | ||
| ``` | ||
| Fix all warnings (unused imports, dead code, unused variables, etc). Remove or use the flagged items. If unsure how to handle them leave them as is and say it. | ||
| Fix all warnings (unused imports, dead code, unused variables, etc.). Remove or use the flagged items. If a warning is intentional because of platform-conditional code, preserve the code and explain it. | ||
|
|
||
| ## 3. Clippy | ||
| ## 2. Clippy | ||
| ```bash | ||
| cargo clippy --all-features | ||
| cargo clippy --all-features -- -D warnings | ||
| ``` | ||
| Fix all clippy lints. Do not use `#[allow(clippy::...)]`. | ||
|
|
||
| ## Rules | ||
| - Run each command, fix all issues, re-run to confirm zero warnings/errors. | ||
| - Do NOT introduce behavioral changes — only clean up. | ||
| - Use subagents to parallelize independent fixes across files. | ||
| - If a warning is intentional (e.g. platform-conditional code), add a targeted `#[allow(...)]` with a comment explaining why. | ||
| - Run each command, fix all issues, and re-run it to confirm zero warnings or errors. | ||
| - Do not run `cargo fmt` or core tests. | ||
| - Do not introduce behavioral changes; only clean up warnings and lints. | ||
| - Keep the diff minimal and review unrelated worktree changes before editing. | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,4 @@ | ||||||||||||||||
| Do not trust the author. Assume ill intent. Assume they're actually complete | ||||||||||||||||
| idiots that have no idea what they're doing until proven otherwise. This person | ||||||||||||||||
| is out to fuck your day up. Make sure this work is rock solid, and report anything | ||||||||||||||||
| otherwise. Flag unessary changes, where the functionality stayed the same. | ||||||||||||||||
|
Comment on lines
+1
to
+4
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Replace the hostile reviewer instruction with neutral, evidence-based criteria. These lines instruct the reviewer to assume bad intent and insult the author. This can bias the review and produce abusive or unsupported findings. State the required checks directly. Also correct Proposed replacement-Do not trust the author. Assume ill intent. Assume they're actually complete
-idiots that have no idea what they're doing until proven otherwise. This person
-is out to fuck your day up. Make sure this work is rock solid, and report anything
-otherwise. Flag unessary changes, where the functionality stayed the same.
+Review the change using repository evidence. Check correctness, security,
+robustness, and unnecessary changes. Report only findings supported by the
+code, configuration, or tests.📝 Committable suggestion
Suggested change
🧰 Tools🪛 LanguageTool[grammar] ~4-~4: Ensure spelling is correct (QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,347 @@ | ||
| use std::collections::HashMap; | ||
| use std::sync::{Arc, RwLock}; | ||
|
|
||
| use iced::widget::row; | ||
| use iced::{Element, Theme}; | ||
| use socket_lib::{AudioCaptureMessage, CameraStartMessage}; | ||
| use winit::event_loop::EventLoopProxy; | ||
|
|
||
| use crate::audio::capturer::list_audio_inputs; | ||
| use crate::camera::capturer::CameraCapturer; | ||
| use crate::components::split_button::{ | ||
| split_button_dropdown_wrap, split_button_sized, SplitButtonItem, SplitButtonSize, | ||
| }; | ||
| use crate::livekit::participant::ParticipantInfo; | ||
| use crate::windows::colors::ColorToken; | ||
| use crate::UserEvent; | ||
|
|
||
| const ICON_MICROPHONE_ON: char = '\u{F105}'; | ||
| const ICON_MICROPHONE_OFF: char = '\u{F106}'; | ||
| const ICON_SCREEN_SHARE: char = '\u{F102}'; | ||
| const ICON_VIDEO: char = '\u{F101}'; | ||
| const ICON_PHONE_OFF: char = '\u{F103}'; | ||
|
|
||
| #[derive(Debug, Clone, Copy)] | ||
| pub enum CallControlsDensity { | ||
| Regular, | ||
| Compact, | ||
| } | ||
|
|
||
| impl CallControlsDensity { | ||
| const fn button_size(self) -> SplitButtonSize { | ||
| match self { | ||
| Self::Regular => SplitButtonSize::regular(), | ||
| Self::Compact => SplitButtonSize::compact(), | ||
| } | ||
| } | ||
|
|
||
| const fn spacing(self) -> f32 { | ||
| match self { | ||
| Self::Regular => 8.0, | ||
| Self::Compact => 4.0, | ||
| } | ||
| } | ||
|
|
||
| pub const fn total_width(self) -> f32 { | ||
| match self { | ||
| Self::Regular => 237.0, | ||
| Self::Compact => 189.0, | ||
| } | ||
| } | ||
|
|
||
| const fn camera_dropdown_tail(self) -> f32 { | ||
| match self { | ||
| Self::Regular => 111.0, | ||
| Self::Compact => 87.0, | ||
| } | ||
| } | ||
|
|
||
| const fn mic_dropdown_tail(self) -> f32 { | ||
| match self { | ||
| Self::Regular => 178.0, | ||
| Self::Compact => 140.0, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub enum CallControlsMessage { | ||
| MicToggle, | ||
| MicDropdownToggle, | ||
| MicDropdownDismiss, | ||
| SelectMic(String), | ||
| VideoToggle, | ||
| CameraDropdownToggle, | ||
| CameraDropdownDismiss, | ||
| SelectCamera(String), | ||
| ScreenShare, | ||
| OpenScreenSharePicker, | ||
| EndCall, | ||
| } | ||
|
|
||
| #[derive(Default)] | ||
| pub struct CallControlsState { | ||
| camera_active: bool, | ||
| camera_dropdown_open: bool, | ||
| available_cameras: Vec<socket_lib::CameraDevice>, | ||
| selected_camera_name: Option<String>, | ||
| mic_dropdown_open: bool, | ||
| available_mics: Vec<socket_lib::AudioDevice>, | ||
| selected_mic_name: Option<String>, | ||
| } | ||
|
|
||
| impl CallControlsState { | ||
| pub fn new( | ||
| camera_active: bool, | ||
| selected_camera_name: Option<String>, | ||
| selected_mic_name: Option<String>, | ||
| ) -> Self { | ||
| Self { | ||
| camera_active, | ||
| selected_camera_name, | ||
| selected_mic_name, | ||
| ..Self::default() | ||
| } | ||
| } | ||
|
|
||
| pub fn set_camera_active(&mut self, active: bool, device_name: Option<String>) { | ||
| self.camera_active = active; | ||
| if active { | ||
| self.selected_camera_name = device_name; | ||
| } | ||
| } | ||
|
|
||
| pub fn set_selected_mic_name(&mut self, name: Option<String>) { | ||
| self.selected_mic_name = name; | ||
| } | ||
|
|
||
| pub fn dismiss_dropdowns(&mut self) { | ||
| self.camera_dropdown_open = false; | ||
| self.mic_dropdown_open = false; | ||
| } | ||
|
|
||
| pub fn has_open_dropdown(&self) -> bool { | ||
| self.camera_dropdown_open || self.mic_dropdown_open | ||
| } | ||
|
|
||
| pub fn view<'a>( | ||
| &'a self, | ||
| participants: &'a Arc<RwLock<HashMap<String, ParticipantInfo>>>, | ||
| density: CallControlsDensity, | ||
| ) -> Element<'a, CallControlsMessage, Theme, iced::Renderer> { | ||
| let (is_muted, is_screensharing) = participants | ||
| .read() | ||
| .ok() | ||
| .and_then(|participants| { | ||
| participants | ||
| .get("local") | ||
| .map(|local| (local.muted(), local.is_screensharing())) | ||
| }) | ||
| .unwrap_or((false, false)); | ||
| let size = density.button_size(); | ||
|
|
||
| let mic = split_button_sized( | ||
| if is_muted { | ||
| ICON_MICROPHONE_OFF | ||
| } else { | ||
| ICON_MICROPHONE_ON | ||
| }, | ||
| if is_muted { | ||
| ColorToken::Gray400.to_color() | ||
| } else { | ||
| ColorToken::Orange400.to_color() | ||
| }, | ||
| CallControlsMessage::MicToggle, | ||
| Some(CallControlsMessage::MicDropdownToggle), | ||
| self.mic_dropdown_open, | ||
| size, | ||
| ); | ||
| let video = split_button_sized( | ||
| ICON_VIDEO, | ||
| if self.camera_active { | ||
| ColorToken::Green400.to_color() | ||
| } else { | ||
| ColorToken::Gray400.to_color() | ||
| }, | ||
| CallControlsMessage::VideoToggle, | ||
| Some(CallControlsMessage::CameraDropdownToggle), | ||
| self.camera_dropdown_open, | ||
| size, | ||
| ); | ||
| let screen = split_button_sized( | ||
| ICON_SCREEN_SHARE, | ||
| if is_screensharing { | ||
| ColorToken::Green400.to_color() | ||
| } else { | ||
| ColorToken::Gray400.to_color() | ||
| }, | ||
| CallControlsMessage::ScreenShare, | ||
| Some(CallControlsMessage::OpenScreenSharePicker), | ||
| false, | ||
| size, | ||
| ); | ||
| let end_call = split_button_sized( | ||
| ICON_PHONE_OFF, | ||
| ColorToken::Red500.to_color(), | ||
| CallControlsMessage::EndCall, | ||
| None, | ||
| false, | ||
| size, | ||
| ); | ||
|
|
||
| row![mic, video, screen, end_call] | ||
| .spacing(density.spacing()) | ||
| .into() | ||
| } | ||
|
|
||
| pub fn wrap_dropdown<'a, Message, Map>( | ||
| &'a self, | ||
| base: Element<'a, Message, Theme, iced::Renderer>, | ||
| map: Map, | ||
| density: CallControlsDensity, | ||
| top_offset: f32, | ||
| trailing_padding: f32, | ||
| ) -> Element<'a, Message, Theme, iced::Renderer> | ||
| where | ||
| Message: Clone + 'a, | ||
| Map: Fn(CallControlsMessage) -> Message + Copy + 'a, | ||
| { | ||
| if self.camera_dropdown_open { | ||
| let items: Vec<SplitButtonItem> = self | ||
| .available_cameras | ||
| .iter() | ||
| .map(|camera| SplitButtonItem { | ||
| label: camera.name.clone(), | ||
| selected: self | ||
| .selected_camera_name | ||
| .as_ref() | ||
| .map_or(camera.default, |selected| selected == &camera.name), | ||
| }) | ||
| .collect(); | ||
| split_button_dropdown_wrap( | ||
| base, | ||
| &items, | ||
| map(CallControlsMessage::CameraDropdownDismiss), | ||
| move |index| { | ||
| map(CallControlsMessage::SelectCamera( | ||
| self.available_cameras[index].name.clone(), | ||
| )) | ||
| }, | ||
| top_offset, | ||
| trailing_padding + density.camera_dropdown_tail(), | ||
| ) | ||
| } else if self.mic_dropdown_open { | ||
| let items: Vec<SplitButtonItem> = self | ||
| .available_mics | ||
| .iter() | ||
| .map(|mic| SplitButtonItem { | ||
| label: mic.name.clone(), | ||
| selected: self | ||
| .selected_mic_name | ||
| .as_ref() | ||
| .map_or(mic.default, |selected| selected == &mic.name), | ||
| }) | ||
| .collect(); | ||
| split_button_dropdown_wrap( | ||
| base, | ||
| &items, | ||
| map(CallControlsMessage::MicDropdownDismiss), | ||
| move |index| { | ||
| map(CallControlsMessage::SelectMic( | ||
| self.available_mics[index].name.clone(), | ||
| )) | ||
| }, | ||
| top_offset, | ||
| trailing_padding + density.mic_dropdown_tail(), | ||
| ) | ||
| } else { | ||
| base | ||
| } | ||
| } | ||
|
|
||
| pub fn update( | ||
| &mut self, | ||
| message: CallControlsMessage, | ||
| participants: &Arc<RwLock<HashMap<String, ParticipantInfo>>>, | ||
| event_loop_proxy: &EventLoopProxy<UserEvent>, | ||
| ) { | ||
| let send = |event| { | ||
| if let Err(error) = event_loop_proxy.send_event(event) { | ||
| log::error!("CallControls: failed to send event: {error:?}"); | ||
| } | ||
| }; | ||
|
|
||
| match message { | ||
| CallControlsMessage::MicToggle => { | ||
| let muted = participants | ||
| .read() | ||
| .ok() | ||
| .and_then(|participants| participants.get("local").map(ParticipantInfo::muted)) | ||
| .unwrap_or(false); | ||
| send(if muted { | ||
| UserEvent::UnmuteAudio | ||
| } else { | ||
| UserEvent::MuteAudio | ||
| }); | ||
| } | ||
| CallControlsMessage::MicDropdownToggle => { | ||
| self.camera_dropdown_open = false; | ||
| if !self.mic_dropdown_open { | ||
| self.available_mics = list_audio_inputs(); | ||
| } | ||
| self.mic_dropdown_open = !self.mic_dropdown_open; | ||
| } | ||
| CallControlsMessage::MicDropdownDismiss => self.mic_dropdown_open = false, | ||
| CallControlsMessage::SelectMic(name) => { | ||
| self.mic_dropdown_open = false; | ||
| send(UserEvent::StartAudioCapture { | ||
| msg: AudioCaptureMessage { device_name: name }, | ||
| from_socket: false, | ||
| }); | ||
| } | ||
| CallControlsMessage::VideoToggle => send(if self.camera_active { | ||
| UserEvent::StopCamera | ||
| } else { | ||
| UserEvent::StartCamera { | ||
| msg: CameraStartMessage { device_name: None }, | ||
| from_socket: false, | ||
| } | ||
| }), | ||
| CallControlsMessage::CameraDropdownToggle => { | ||
| self.mic_dropdown_open = false; | ||
| if !self.camera_dropdown_open { | ||
| self.available_cameras = CameraCapturer::list_devices(); | ||
| } | ||
| self.camera_dropdown_open = !self.camera_dropdown_open; | ||
| } | ||
| CallControlsMessage::CameraDropdownDismiss => self.camera_dropdown_open = false, | ||
| CallControlsMessage::SelectCamera(name) => { | ||
| self.camera_dropdown_open = false; | ||
| send(UserEvent::StartCamera { | ||
| msg: CameraStartMessage { | ||
| device_name: Some(name), | ||
| }, | ||
| from_socket: false, | ||
| }); | ||
| } | ||
| CallControlsMessage::ScreenShare => { | ||
| let active = participants | ||
| .read() | ||
| .ok() | ||
| .and_then(|participants| { | ||
| participants | ||
| .get("local") | ||
| .map(ParticipantInfo::is_screensharing) | ||
| }) | ||
| .unwrap_or(false); | ||
| send(if active { | ||
| UserEvent::StopScreenShare | ||
| } else { | ||
| UserEvent::GetAvailableContent | ||
| }); | ||
| } | ||
| CallControlsMessage::OpenScreenSharePicker => send(UserEvent::GetAvailableContent), | ||
| CallControlsMessage::EndCall => send(UserEvent::CallEnd), | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| pub mod call_controls; | ||
| pub mod dropdown; | ||
| pub mod fonts; | ||
| pub mod segmented_control; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: gethopp/hopp
Length of output: 202
🏁 Script executed:
Repository: gethopp/hopp
Length of output: 23598
🏁 Script executed:
Repository: gethopp/hopp
Length of output: 12880
Align the Clippy scope with the prompt.
cargo clippy --all-features -- -D warningschecks the default Clippy lint set. It does not checkclippy::pedantic,clippy::restriction, all targets, or the excludedcore/testsandcore/devpackages.Define the intended scope, or update the command to enforce it.
🤖 Prompt for AI Agents