Skip to content

fix: avoid js error on output change #67

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

Closed
tonyco97 wants to merge 4 commits into from
Closed

fix: avoid js error on output change #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dae86cd
fix: avoid js error on output change
tonyco97 Jun 26, 2025
a2a177a
fix: prevent infinite re-render loop in audio output device selection
edospadoni Jun 27, 2025
edf4ca5
fix: onclick can be empty
edospadoni Jun 27, 2025
2b1de09
fix: avoid re-renders
edospadoni Jun 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
196 changes: 123 additions & 73 deletions ...nts/Modules/NethVoice/BaseModule/ProfileDialog/SettingsSettings/SettingsDevicesDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export function SettingsDeviceDialog() {
videoInput: z.string(),
})

const { handleSubmit, control, setValue } = useForm({
const { handleSubmit, control, setValue, watch } = useForm({
defaultValues: {
audioInput: '',
audioOutput: '',
Expand All @@ -82,6 +82,8 @@ export function SettingsDeviceDialog() {
resolver: zodResolver(schema),
})

const formValues = watch()

useEffect(() => {
initDevices()
}, [])
Expand All @@ -101,10 +103,21 @@ export function SettingsDeviceDialog() {
return storeValue || ''
}

setValue('audioInput', getEffectiveValue('audioInput'))
setValue('audioOutput', getEffectiveValue('audioOutput'))
setValue('videoInput', getEffectiveValue('videoInput'))
}, [account?.preferredDevices])
const audioInputValue = getEffectiveValue('audioInput')
const audioOutputValue = getEffectiveValue('audioOutput')
const videoInputValue = getEffectiveValue('videoInput')

// Only set values if they are actually different to prevent unnecessary re-renders
if (formValues.audioInput !== audioInputValue) {
setValue('audioInput', audioInputValue, { shouldDirty: false })
}
if (formValues.audioOutput !== audioOutputValue) {
setValue('audioOutput', audioOutputValue, { shouldDirty: false })
}
if (formValues.videoInput !== videoInputValue) {
setValue('videoInput', videoInputValue, { shouldDirty: false })
}
}, [account?.preferredDevices, setValue, formValues.audioInput, formValues.audioOutput, formValues.videoInput])

useEffect(() => {
const handleStorageChange = (e: StorageEvent) => {
Expand All @@ -116,7 +129,13 @@ export function SettingsDeviceDialog() {

if (changedDeviceType && e.newValue) {
console.log(`localStorage changed for ${changedDeviceType}:`, e.newValue)
setValue(changedDeviceType, e.newValue)
try {
const parsed = JSON.parse(e.newValue)
const deviceId = parsed.deviceId || e.newValue
setValue(changedDeviceType, deviceId, { shouldDirty: false })
} catch {
setValue(changedDeviceType, e.newValue, { shouldDirty: false })
}
}
}

Expand All @@ -126,30 +145,40 @@ export function SettingsDeviceDialog() {

const getDeviceById = useCallback(
(type: DeviceType, id: string): MediaDeviceInfo | undefined => {
if (!devices) return undefined
return devices[type].find((d) => d.deviceId === id)
if (!devices || !devices[type] || !id) return undefined
return devices[type].find((d) => d && d.deviceId === id)
},
[devices],
)

const initDevices = async () => {
const devices = await getMediaDevices()
Log.info('Available devices:', devices)
setDevices(devices)
if (devices?.audioOutput) {
Log.info('Audio output device IDs:', devices.audioOutput.map(d => ({ id: d.deviceId, label: d.label })))
}
if (devices) {
setDevices(devices)
}
}

async function getMediaDevices() {
try {
if (!navigator.mediaDevices || !navigator.mediaDevices.enumerateDevices) {
console.warn('Media devices API not available')
return { audioInput: [], audioOutput: [], videoInput: [] }
}

const devices = await navigator.mediaDevices.enumerateDevices()

const audioInput = devices.filter((device) => device.kind === 'audioinput')
const audioOutput = devices.filter((device) => device.kind === 'audiooutput')
const videoInput = devices.filter((device) => device.kind === 'videoinput')
const audioInput = devices.filter((device) => device && device.kind === 'audioinput')
const audioOutput = devices.filter((device) => device && device.kind === 'audiooutput')
const videoInput = devices.filter((device) => device && device.kind === 'videoinput')

return { audioInput, audioOutput, videoInput }
} catch (err) {
console.error('Error reading audio and video devices:', err)
return null
return { audioInput: [], audioOutput: [], videoInput: [] }
}
}

Expand Down Expand Up @@ -197,30 +226,51 @@ export function SettingsDeviceDialog() {
<FontAwesomeIcon icon={icons[name]} className='w-4' />
<span className='truncate'>{fieldLabels[name]}</span>
</div>
<div className=''>
<div>
<Controller
control={control}
name={name}
render={({ field: { value, onChange } }) => {
const selectedDevice = getDeviceById(name, value)

// If selected device is not found in current devices list, it might have been disconnected
if (value && !selectedDevice && devices?.[name] && devices[name].length > 0) {
console.warn(`[${name}] Selected device ${value} not found in current devices, device may have been disconnected`)
}
return (
<Dropdown
items={devices?.[name].map((device) => {
items={devices?.[name]?.map((device) => {
if (!device) return null
return (
<DropdownItem
key={device.deviceId}
onClick={() => {
console.log('change device:', device.deviceId)
onChange(device.deviceId)
key={device.deviceId || `device-${Math.random()}`}
onClick={(e) => {
e?.preventDefault()
e?.stopPropagation()

if (value !== device.deviceId && device.deviceId) {
console.log(`[${name}] change device:`, device.deviceId, 'current:', value)
console.log(`[${name}] device label:`, device.label)

// Use setTimeout to prevent potential re-render loops and add extra validation
setTimeout(() => {
// Double check the value hasn't changed in the meantime
if (device.deviceId && device.deviceId !== value) {
onChange(device.deviceId)
}
}, 0)
} else {
console.log(`[${name}] clicked same device, ignoring:`, device.deviceId)
}
}}
>
<div className='flex flex-row items-center gap-2 w-[200px]'>
<span
className='truncate'
data-tooltip-id={`device-${name}`}
data-tooltip-content={device.label}
data-tooltip-content={device.label || 'Unknown device'}
>
{device.label}
{device.label || 'Unknown device'}
</span>
<FontAwesomeIcon
icon={SelectedIcon}
Expand All @@ -233,7 +283,7 @@ export function SettingsDeviceDialog() {
</div>
</DropdownItem>
)
})}
}).filter(Boolean) || []}
className='w-full'
>
<DropdownHeader>
Expand Down Expand Up @@ -265,7 +315,7 @@ export function SettingsDeviceDialog() {
</div>
)
},
[account?.preferredDevices, devices],
[devices, icons, fieldLabels, control, getDeviceById],
)

return (
Expand All @@ -281,58 +331,58 @@ export function SettingsDeviceDialog() {

<div className='fixed inset-0 z-[205] overflow-y-auto pointer-events-none'>
<div className='flex min-h-full items-center justify-center p-4 pointer-events-none'>
<div className='bg-bgLight dark:bg-bgDark text-bgDark dark:text-bgLight rounded-xl shadow-lg max-w-sm w-full pointer-events-auto'>
{/* Dialog content */}
<div className='p-6 flex flex-col gap-4'>
{/* Title */}
<h2 className='text-center font-semibold text-xl'>
{t('TopBar.Preferred devices')}
</h2>

{/* Form */}
<form
onSubmit={handleSubmit(submit)}
className='flex flex-col gap-1'
>
{/* Input field with clear button next to it */}
<DeviceDropdown name='audioInput' />
<DeviceDropdown name='audioOutput' />
<DeviceDropdown name='videoInput' />

{/* Inline notification */}
{isDeviceUnavailable && (
<InlineNotification
title={t('Common.Warning')}
type='warning'
className=''
>
<p>{t('Devices.Inline warning message devices')}</p>
</InlineNotification>
)}
{/* Action buttons */}
<div className='flex flex-col gap-3 mt-2'>
<Button
variant='primary'
type='submit'
className='w-full py-3 rounded-lg font-medium'
disabled={isDeviceUnavailable}
>
{t('Common.Save')}
</Button>

<Button
variant='ghost'
type='button'
onClick={handleCancel}
className='text-center text-blue-700 dark:text-blue-500 font-medium'
>
{t('Common.Cancel')}
</Button>
</div>
</form>
<div className='bg-bgLight dark:bg-bgDark text-bgDark dark:text-bgLight rounded-xl shadow-lg max-w-sm w-full pointer-events-auto'>
{/* Dialog content */}
<div className='p-6 flex flex-col gap-4'>
{/* Title */}
<h2 className='text-center font-semibold text-xl'>
{t('TopBar.Preferred devices')}
</h2>

{/* Form */}
<form
onSubmit={handleSubmit(submit)}
className='flex flex-col gap-1'
>
{/* Input field with clear button next to it */}
<DeviceDropdown name='audioInput' />
<DeviceDropdown name='audioOutput' />
<DeviceDropdown name='videoInput' />

{/* Inline notification */}
{isDeviceUnavailable && (
<InlineNotification
title={t('Common.Warning')}
type='warning'
className=''
>
<p>{t('Devices.Inline warning message devices')}</p>
</InlineNotification>
)}
{/* Action buttons */}
<div className='flex flex-col gap-3 mt-2'>
<Button
variant='primary'
type='submit'
className='w-full py-3 rounded-lg font-medium'
disabled={isDeviceUnavailable}
>
{t('Common.Save')}
</Button>

<Button
variant='ghost'
type='button'
onClick={handleCancel}
className='text-center text-blue-700 dark:text-blue-500 font-medium'
>
{t('Common.Cancel')}
</Button>
</div>
</form>
</div>
</div>
</div>
</div>
</div>
</>
)
Expand Down
2 changes: 1 addition & 1 deletion src/renderer/src/components/Nethesis/dropdown/DropdownItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'
import { IconDefinition } from '@fortawesome/fontawesome-common-types'

export interface DropdownItemProps extends Omit<ComponentProps<'div'>, 'className'> {
onClick?: () => void
onClick?: (e?: React.MouseEvent<HTMLDivElement>) => void
icon?: IconDefinition
centered?: boolean
variantTop?: boolean
Expand Down