From ed7ee3d9f8d8d8a5a96b6ba012b807fefa494764 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Thu, 17 Apr 2025 19:23:53 -0400 Subject: [PATCH] fix(ui): always order album tracks by disc and track number (fixes #3720) (#3975) * fix(ui): ensure album tracks are always ordered by disc and track number (fixes #3720) * refactor(ui): remove obsolete release date grouping logic from SongDatagrid and AlbumSongs * fix(ui): ensure correct album track ordering in context menu and play button * fix: Update album sort to use album_id instead of release_date * refactor: Adjust filters in PlayButton and AlbumContextMenu * fix: correct typo in comment regarding participants in GetMissingAndMatching function * fix: prevent visual separation of tracks on same disc Removes the leftover `releaseDate` check from the `firstTracksOfDiscs` calculation in `SongDatagridBody`. This check caused unnecessary `DiscSubtitleRow` insertions when tracks on the same disc had different release dates, leading to an incorrect visual grouping that resembled a multi-disc layout. This change ensures disc subtitles are only shown when the actual `discNumber` changes, correcting the UI presentation issue reported in issue #3720 after PR #3975. * fix: remove remaining releaseDate references in SongDatagrid Cleaned up leftover `releaseDate` references in `SongDatagrid.jsx`: - Removed `releaseDate` parameter and usage from `handlePlaySubset` in `DiscSubtitleRow`. - Removed `releaseDate` prop passed to `AlbumContextMenu` in `DiscSubtitleRow`. - Removed `releaseDate` from the drag item data in `SongDatagridRow`. - Removed `releaseDate` parameter and the corresponding `else` block from the `playSubset` function in `SongDatagridBody`. This ensures the component consistently uses `discNumber` for grouping and playback actions initiated from the disc subtitle, fully resolving the inconsistencies related to issue #3720. --- persistence/mediafile_repository.go | 4 +- ui/src/album/AlbumSongs.jsx | 1 - ui/src/common/ContextMenus.jsx | 1 - ui/src/common/PlayButton.jsx | 1 - ui/src/common/SongDatagrid.jsx | 108 ++-------------------------- 5 files changed, 8 insertions(+), 107 deletions(-) diff --git a/persistence/mediafile_repository.go b/persistence/mediafile_repository.go index ebf07ce17..a86c8f668 100644 --- a/persistence/mediafile_repository.go +++ b/persistence/mediafile_repository.go @@ -77,7 +77,7 @@ func NewMediaFileRepository(ctx context.Context, db dbx.Builder) model.MediaFile "title": "order_title", "artist": "order_artist_name, order_album_name, release_date, disc_number, track_number", "album_artist": "order_album_artist_name, order_album_name, release_date, disc_number, track_number", - "album": "order_album_name, release_date, disc_number, track_number, order_artist_name, title", + "album": "order_album_name, album_id, disc_number, track_number, order_artist_name, title", "random": "random", "created_at": "media_file.created_at", "starred_at": "starred, starred_at", @@ -242,7 +242,7 @@ func (r *mediaFileRepository) MarkMissingByFolder(missing bool, folderIDs ...str // GetMissingAndMatching returns all mediafiles that are missing and their potential matches (comparing PIDs) // that were added/updated after the last scan started. The result is ordered by PID. -// It does not need to load bookmarks, annotations and participnts, as they are not used by the scanner. +// It does not need to load bookmarks, annotations and participants, as they are not used by the scanner. func (r *mediaFileRepository) GetMissingAndMatching(libId int) (model.MediaFileCursor, error) { subQ := r.newSelect().Columns("pid"). Where(And{ diff --git a/ui/src/album/AlbumSongs.jsx b/ui/src/album/AlbumSongs.jsx index bfb1a4d6a..d705617e1 100644 --- a/ui/src/album/AlbumSongs.jsx +++ b/ui/src/album/AlbumSongs.jsx @@ -185,7 +185,6 @@ const AlbumSongs = (props) => { {...props} hasBulkActions={true} showDiscSubtitles={true} - showReleaseDivider={true} contextAlwaysVisible={!isDesktop} classes={{ row: classes.row }} > diff --git a/ui/src/common/ContextMenus.jsx b/ui/src/common/ContextMenus.jsx index 855825496..47c9c6786 100644 --- a/ui/src/common/ContextMenus.jsx +++ b/ui/src/common/ContextMenus.jsx @@ -231,7 +231,6 @@ export const AlbumContextMenu = (props) => sort: { field: 'album', order: 'ASC' }, filter: { album_id: props.record.id, - release_date: props.releaseDate, disc_number: props.discNumber, missing: false, }, diff --git a/ui/src/common/PlayButton.jsx b/ui/src/common/PlayButton.jsx index 04b36ef5b..d8717236f 100644 --- a/ui/src/common/PlayButton.jsx +++ b/ui/src/common/PlayButton.jsx @@ -24,7 +24,6 @@ export const PlayButton = ({ record, size, className }) => { sort: { field: 'album', order: 'ASC' }, filter: { album_id: record.id, - release_date: record.releaseDate, disc_number: record.discNumber, }, }) diff --git a/ui/src/common/SongDatagrid.jsx b/ui/src/common/SongDatagrid.jsx index 78f580e67..3586cf225 100644 --- a/ui/src/common/SongDatagrid.jsx +++ b/ui/src/common/SongDatagrid.jsx @@ -59,59 +59,12 @@ const useStyles = makeStyles({ }, }) -const ReleaseRow = forwardRef( - ({ record, onClick, colSpan, contextAlwaysVisible }, ref) => { - const isDesktop = useMediaQuery((theme) => theme.breakpoints.up('md')) - const classes = useStyles({ isDesktop }) - const translate = useTranslate() - const handlePlaySubset = (releaseDate) => () => { - onClick(releaseDate) - } - - let releaseTitle = [] - if (record.releaseDate) { - releaseTitle.push(translate('resources.album.fields.released')) - releaseTitle.push(formatFullDate(record.releaseDate)) - if (record.catalogNum && isDesktop) { - releaseTitle.push('ยท Cat #') - releaseTitle.push(record.catalogNum) - } - } - - return ( - - - - {releaseTitle.join(' ')} - - - - - - - ) - }, -) - -ReleaseRow.displayName = 'ReleaseRow' - const DiscSubtitleRow = forwardRef( ({ record, onClick, colSpan, contextAlwaysVisible }, ref) => { const isDesktop = useMediaQuery((theme) => theme.breakpoints.up('md')) const classes = useStyles({ isDesktop }) - const handlePlaySubset = (releaseDate, discNumber) => () => { - onClick(releaseDate, discNumber) + const handlePlaySubset = (discNumber) => () => { + onClick(discNumber) } let subtitle = [] @@ -126,7 +79,7 @@ const DiscSubtitleRow = forwardRef( @@ -139,7 +92,6 @@ const DiscSubtitleRow = forwardRef( - {firstTracksOfReleases.has(record.id) && ( - - )} {firstTracksOfDiscs.has(record.id) && ( { const dispatch = useDispatch() const { ids, data } = rest const playSubset = useCallback( - (releaseDate, discNumber) => { + (discNumber) => { let idsToPlay = [] if (discNumber !== undefined) { - idsToPlay = ids.filter( - (id) => - data[id].releaseDate === releaseDate && - data[id].discNumber === discNumber, - ) - } else { - idsToPlay = ids.filter((id) => data[id].releaseDate === releaseDate) + idsToPlay = ids.filter((id) => data[id].discNumber === discNumber) } dispatch( playTracks( @@ -297,8 +230,7 @@ const SongDatagridBody = ({ foundSubtitle = foundSubtitle || data[id].discSubtitle if ( acc.length === 0 || - (last && data[id].discNumber !== data[last].discNumber) || - (last && data[id].releaseDate !== data[last].releaseDate) + (last && data[id].discNumber !== data[last].discNumber) ) { acc.push(id) } @@ -311,37 +243,12 @@ const SongDatagridBody = ({ return set }, [ids, data, showDiscSubtitles]) - const firstTracksOfReleases = useMemo(() => { - if (!ids) { - return new Set() - } - const set = new Set( - ids - .filter((i) => data[i]) - .reduce((acc, id) => { - const last = acc && acc[acc.length - 1] - if ( - acc.length === 0 || - (last && data[id].releaseDate !== data[last].releaseDate) - ) { - acc.push(id) - } - return acc - }, []), - ) - if (!showReleaseDivider || set.size < 2) { - set.clear() - } - return set - }, [ids, data, showReleaseDivider]) - return ( @@ -353,7 +260,6 @@ const SongDatagridBody = ({ export const SongDatagrid = ({ contextAlwaysVisible, showDiscSubtitles, - showReleaseDivider, ...rest }) => { const classes = useStyles() @@ -366,7 +272,6 @@ export const SongDatagrid = ({ } /> @@ -376,6 +281,5 @@ export const SongDatagrid = ({ SongDatagrid.propTypes = { contextAlwaysVisible: PropTypes.bool, showDiscSubtitles: PropTypes.bool, - showReleaseDivider: PropTypes.bool, classes: PropTypes.object, }