Skip to content

feat - show configured audio devices on connect screen (#350) - #351

Open
JustusPlays78 wants to merge 2 commits into
pierr3:mainfrom
JustusPlays78:feature/350-show-audio-hardware-on-connect-screen
Open

feat - show configured audio devices on connect screen (#350)#351
JustusPlays78 wants to merge 2 commits into
pierr3:mainfrom
JustusPlays78:feature/350-show-audio-hardware-on-connect-screen

Conversation

@JustusPlays78

@JustusPlays78 JustusPlays78 commented May 30, 2026

Copy link
Copy Markdown
image image image

Tested it only with mock data as i was not able to build everything from that repo completetly local.

Feel free to edit or request changes

@JustusPlays78

Copy link
Copy Markdown
Author

any progress here?

@JustusPlays78

Copy link
Copy Markdown
Author

@pierr3 ^^

@pierr3
pierr3 requested review from Copilot and pierr3 July 22, 2026 16:03
@pierr3 pierr3 self-assigned this Jul 22, 2026
@pierr3 pierr3 added the enhancement New feature or request label Jul 22, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a small “configured audio devices” summary to the pre-connect radio screen so users can immediately see whether their microphone/headset/speaker selections are configured and available before connecting to the VATSIM audio network.

Changes:

  • Adds a new AudioHardwareSummary UI component that resolves configured device IDs to human-readable device names.
  • Renders the summary on the “waiting for connection” screen in the radio container.
  • Dispatches an app-level event from the settings modal when audio configuration is changed to trigger refresh.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.

File Description
src/renderer/src/components/settings-modal/settings-modal.tsx Dispatches an “audio config changed” window event after settings changes so the connect screen summary can refresh.
src/renderer/src/components/radio/radio-container.tsx Renders the new audio hardware summary on the waiting-for-connection UI.
src/renderer/src/components/radio/audio-hardware-summary.tsx New component that fetches config + audio device lists and displays configured/available status.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/renderer/src/components/settings-modal/settings-modal.tsx
Comment thread src/renderer/src/components/settings-modal/settings-modal.tsx
Comment thread src/renderer/src/components/settings-modal/settings-modal.tsx
Comment thread src/renderer/src/components/settings-modal/settings-modal.tsx
Comment thread src/renderer/src/components/settings-modal/settings-modal.tsx
Comment thread src/renderer/src/components/radio/audio-hardware-summary.tsx
Comment thread src/renderer/src/components/radio/audio-hardware-summary.tsx
Comment thread src/renderer/src/components/radio/audio-hardware-summary.tsx
Comment thread src/renderer/src/components/radio/audio-hardware-summary.tsx
Comment thread src/renderer/src/components/radio/audio-hardware-summary.tsx

@pierr3 pierr3 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the contribution — the feature itself is a good idea and the UI looks right. However, I'd like this restructured before merging, because the update mechanism introduces a pattern that doesn't exist anywhere else in the codebase.

Main feedback: drop the custom DOM event (window.dispatchEvent / AUDIO_CONFIG_CHANGED_EVENT) and the focus-listener refresh. This repo has an established pattern for pushing state changes to components: IPC events from the main process are subscribed once in src/renderer/src/interfaces/IPCInterface.ts, which writes into a Zustand store, and components read reactively from the store (see how main-volume-change works, for example). The custom window event creates a parallel, renderer-only event system, couples the settings modal to a constant exported from a UI component, and has a race (the event fires before the window.api.set… invokes actually settle, so the summary can refresh against stale config — Copilot flagged this too).

Suggested structure:

  1. In src/main/index.ts, have the set-audio-api / set-audio-input-device / set-headset-output-device / set-speaker-output-device handlers send a config-changed event to the renderer (with the updated config) after updating the config — same as main-volume-change does today.
  2. Subscribe to it once in IPCInterface.ts and push the relevant fields into a store (utilStore or a small config store).
  3. Have AudioHardwareSummary read from the store and re-resolve device names when it changes. No window events, no focus listener, no refresh plumbing in the component.

A few smaller things that still apply after the restructure:

  • On fetch errors the lines stay stuck in the "loading" state forever — set an explicit error/unavailable state instead.
  • gap-0.5 isn't a Bootstrap class, so it's a no-op — use gap-1/gap-2 or a custom style.
  • Import Configuration the same way other radio components do rather than the deep relative path.
  • The settings modal already has device-validity logic (isInputDeviceValid etc.) — once config lives in a store, the "unavailable" check could be shared instead of duplicated in resolveDeviceStatus.

Happy to help with the IPC wiring if the build setup is the blocker — the backend build isn't needed to run the renderer against the existing native package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants