Skip to content

Conversation

@jrobinso
Copy link
Contributor

@jrobinso jrobinso commented Dec 9, 2025

No description provided.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements track selection restoration functionality by introducing a new list-based modal for track selection alongside the existing checkbox-based modal. The changes distinguish between track hub selections (which use checkboxes) and registry-based track selections (which use a select list).

Key changes:

  • Renamed the _checked property to _loaded to better reflect that it represents the loaded state of tracks in the browser, not just UI selection state
  • Added a new trackSelectionListModal component that presents tracks as a multi-select list instead of checkboxes
  • Introduced type field to track configuration objects to determine which modal to display ('hub' uses checkbox modal, others use list modal)

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
js/widgets/trackWidgets.js Imports new list modal, renames _checked to _loaded, adds conditional logic to select modal type based on config.type, adds type field to configuration objects
js/widgets/trackSelectionModal.js Updates to use _loaded property instead of _checked for determining checkbox state
js/widgets/trackSelectionListModal.js New file implementing a select list-based track selection modal (add-only functionality, cannot unload tracks)
js/widgets/genericSelectModal.js Removed unused file

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

console.log('Modal closed with OK button')
const selectedOptions = Array.from(modalElement.querySelectorAll('#track-select option:checked'))
const checkedTracks = selectedOptions.map(option => trackMap.get(option.value))
const uncheckedTracks = [] // There is no way to unselect a track with this widget
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The uncheckedTracks array is always empty, but the okHandler in trackWidgets.js (lines 247-252) expects to process unchecked tracks to remove them from the browser. This means users cannot unload tracks using this modal, which differs from trackSelectionModal where checkboxes can be unchecked. This is a functional limitation that should be addressed. Consider collecting all non-selected tracks (tracks not in the selected options) as uncheckedTracks if track removal is intended to be supported.

Suggested change
const uncheckedTracks = [] // There is no way to unselect a track with this widget
const selectedIds = new Set(selectedOptions.map(option => option.value))
const uncheckedTracks = tracks.filter(track => !selectedIds.has(track._id))

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@jrobinso jrobinso merged commit b046ee3 into master Dec 9, 2025
2 checks passed
@jrobinso jrobinso deleted the list_trackselect branch December 9, 2025 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants