mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-22 07:15:47 +00:00
fix(sidebar): reorder sidebar nav items by id to stop drag drift (#1206)
* fix(sidebar): reorder nav items by id instead of positional index The Settings sidebar customizer rendered library rows with one filter and reordered them with another, so a hidden/gated item (luckyMix when AudioMuse is unavailable) made the rendered list shorter than the reorder list and drag indices landed one slot off — rows jumped back on drop. Replace the index-based reorder with an id-based primitive shared by the customizer and the in-sidebar long-press DnD, so a render filter can no longer desync the move. Unknown, cross-section, conserved, or no-op drops are rejected. * docs(changelog): note sidebar reorder fix (#1206)
This commit is contained in:
@@ -147,11 +147,6 @@ export default function Sidebar({
|
||||
// React Compiler refs rule: ref kept in sync with the latest value for use in effects/handlers/cleanup; not render data.
|
||||
// eslint-disable-next-line react-hooks/refs
|
||||
sidebarItemsRef.current = sidebarItems;
|
||||
const randomNavModeRef = useRef(randomNavMode);
|
||||
// React Compiler refs rule: ref kept in sync with the latest value for use in effects/handlers/cleanup; not render data.
|
||||
// eslint-disable-next-line react-hooks/refs
|
||||
randomNavModeRef.current = randomNavMode;
|
||||
|
||||
const {
|
||||
navDnd,
|
||||
navDndTrashHint,
|
||||
@@ -161,7 +156,6 @@ export default function Sidebar({
|
||||
} = useSidebarNavDnd({
|
||||
isCollapsed,
|
||||
sidebarItemsRef,
|
||||
randomNavModeRef,
|
||||
setSidebarItems,
|
||||
});
|
||||
const newReleasesUnreadCount = useSidebarNewReleasesUnread({
|
||||
@@ -257,9 +251,7 @@ export default function Sidebar({
|
||||
musicFolders={musicFolders}
|
||||
pickLibrary={pickLibrary}
|
||||
visibleLibraryConfigs={visibleLibraryConfigs}
|
||||
libraryItemsForReorder={libraryItemsForReorder}
|
||||
visibleSystemConfigs={visibleSystemConfigs}
|
||||
systemItemsForReorder={systemItemsForReorder}
|
||||
playlistsExpanded={playlistsExpanded}
|
||||
setPlaylistsExpanded={setPlaylistsExpanded}
|
||||
playlists={playlists}
|
||||
|
||||
@@ -6,16 +6,16 @@ import { useAuthStore } from '../../store/authStore';
|
||||
import { useSidebarStore, SidebarItemConfig, CONSERVED_SIDEBAR_NAV_IDS } from '../../store/sidebarStore';
|
||||
import { useLuckyMixAvailable } from '../../hooks/useLuckyMixAvailable';
|
||||
import { ALL_NAV_ITEMS } from '../../config/navItems';
|
||||
import { applySidebarDropReorder } from '../../utils/componentHelpers/sidebarNavReorder';
|
||||
import { applySidebarReorderById } from '../../utils/componentHelpers/sidebarNavReorder';
|
||||
import { SettingsGroup } from './SettingsGroup';
|
||||
import { SettingsToggle } from './SettingsToggle';
|
||||
|
||||
type DropTarget = { idx: number; before: boolean; section: 'library' | 'system' } | null;
|
||||
type DropTarget = { id: string; before: boolean; section: 'library' | 'system' } | null;
|
||||
|
||||
function SidebarGripHandle({ idx, section, label }: { idx: number; section: 'library' | 'system'; label: string }) {
|
||||
function SidebarGripHandle({ id, section, label }: { id: string; section: 'library' | 'system'; label: string }) {
|
||||
const { t } = useTranslation();
|
||||
const { onMouseDown } = useDragSource(() => ({
|
||||
data: JSON.stringify({ type: 'sidebar_reorder', index: idx, section }),
|
||||
data: JSON.stringify({ type: 'sidebar_reorder', id, section }),
|
||||
label,
|
||||
}));
|
||||
return (
|
||||
@@ -72,47 +72,48 @@ export function SidebarCustomizer() {
|
||||
const onPsyDrop = (e: Event) => {
|
||||
const detail = (e as CustomEvent).detail;
|
||||
if (!detail?.data) return;
|
||||
let parsed: { type?: string; index?: number; section?: string };
|
||||
let parsed: { type?: string; id?: string; section?: string };
|
||||
try { parsed = JSON.parse(detail.data); } catch { return; }
|
||||
if (parsed.type !== 'sidebar_reorder' || parsed.index == null || !parsed.section) return;
|
||||
if (parsed.type !== 'sidebar_reorder' || !parsed.id || !parsed.section) return;
|
||||
|
||||
const fromIdx = parsed.index;
|
||||
const draggedId = parsed.id;
|
||||
const fromSection = parsed.section as 'library' | 'system';
|
||||
const target = dropTargetRef.current;
|
||||
dropTargetRef.current = null; setDropTarget(null);
|
||||
|
||||
const next = applySidebarDropReorder(itemsRef.current, fromSection, fromIdx, target, randomNavMode);
|
||||
const next = applySidebarReorderById(itemsRef.current, fromSection, draggedId, target);
|
||||
if (next) setItems(next);
|
||||
};
|
||||
el.addEventListener('psy-drop', onPsyDrop);
|
||||
return () => el.removeEventListener('psy-drop', onPsyDrop);
|
||||
}, [libraryItems, systemItems, setItems, randomNavMode]);
|
||||
}, [setItems]);
|
||||
|
||||
const handleMouseMove = (e: React.MouseEvent) => {
|
||||
if (!isPsyDragging || !containerRef.current) return;
|
||||
const rows = containerRef.current.querySelectorAll<HTMLElement>('[data-sidebar-idx]');
|
||||
const rows = containerRef.current.querySelectorAll<HTMLElement>('[data-sidebar-id]');
|
||||
let target: DropTarget = null;
|
||||
for (const row of rows) {
|
||||
const rect = row.getBoundingClientRect();
|
||||
const idx = Number(row.dataset.sidebarIdx);
|
||||
const id = row.dataset.sidebarId;
|
||||
const section = row.dataset.sidebarSection as 'library' | 'system';
|
||||
if (e.clientY < rect.top + rect.height / 2) { target = { idx, before: true, section }; break; }
|
||||
target = { idx, before: false, section };
|
||||
if (!id) continue;
|
||||
if (e.clientY < rect.top + rect.height / 2) { target = { id, before: true, section }; break; }
|
||||
target = { id, before: false, section };
|
||||
}
|
||||
dropTargetRef.current = target;
|
||||
setDropTarget(target);
|
||||
};
|
||||
|
||||
const renderRow = (cfg: SidebarItemConfig, localIdx: number, section: 'library' | 'system') => {
|
||||
const renderRow = (cfg: SidebarItemConfig, section: 'library' | 'system') => {
|
||||
const meta = ALL_NAV_ITEMS[cfg.id];
|
||||
if (!meta) return null;
|
||||
const Icon = meta.icon;
|
||||
const isBefore = isPsyDragging && dropTarget?.section === section && dropTarget.idx === localIdx && dropTarget.before;
|
||||
const isAfter = isPsyDragging && dropTarget?.section === section && dropTarget.idx === localIdx && !dropTarget.before;
|
||||
const isBefore = isPsyDragging && dropTarget?.section === section && dropTarget.id === cfg.id && dropTarget.before;
|
||||
const isAfter = isPsyDragging && dropTarget?.section === section && dropTarget.id === cfg.id && !dropTarget.before;
|
||||
return (
|
||||
<div
|
||||
key={cfg.id}
|
||||
data-sidebar-idx={localIdx}
|
||||
data-sidebar-id={cfg.id}
|
||||
data-sidebar-section={section}
|
||||
className="sidebar-customizer-row"
|
||||
style={{
|
||||
@@ -120,7 +121,7 @@ export function SidebarCustomizer() {
|
||||
borderBottom: isAfter ? '2px solid var(--accent)' : undefined,
|
||||
}}
|
||||
>
|
||||
<SidebarGripHandle idx={localIdx} section={section} label={t(meta.labelKey)} />
|
||||
<SidebarGripHandle id={cfg.id} section={section} label={t(meta.labelKey)} />
|
||||
<Icon size={16} style={{ color: 'var(--text-muted)', flexShrink: 0 }} />
|
||||
<span style={{ flex: 1, fontSize: 14 }}>{t(meta.labelKey)}</span>
|
||||
<label className="toggle-switch" aria-label={t(meta.labelKey)}>
|
||||
@@ -160,12 +161,12 @@ export function SidebarCustomizer() {
|
||||
{/* Library block */}
|
||||
<div style={{ padding: '4px 0' }}>
|
||||
<div className="sidebar-customizer-block-label">{t('sidebar.library')}</div>
|
||||
{libraryItems.map((cfg, i) => renderRow(cfg, i, 'library'))}
|
||||
{libraryItems.map(cfg => renderRow(cfg, 'library'))}
|
||||
</div>
|
||||
{/* System block */}
|
||||
<div style={{ padding: '4px 0' }}>
|
||||
<div className="sidebar-customizer-block-label">{t('sidebar.system')}</div>
|
||||
{systemItems.map((cfg, i) => renderRow(cfg, i, 'system'))}
|
||||
{systemItems.map(cfg => renderRow(cfg, 'system'))}
|
||||
<div className="sidebar-customizer-fixed-hint">
|
||||
<span>{t('settings.sidebarFixed')}: {t('sidebar.nowPlaying')}, {t('sidebar.settings')}</span>
|
||||
</div>
|
||||
|
||||
@@ -12,7 +12,7 @@ import SidebarActiveJobs from './SidebarActiveJobs';
|
||||
|
||||
interface NavDndState {
|
||||
section: 'library' | 'system';
|
||||
fromIdx: number;
|
||||
draggedId: string;
|
||||
}
|
||||
|
||||
interface MusicFolder { id: string; name: string }
|
||||
@@ -29,17 +29,15 @@ interface Props {
|
||||
musicFolders: MusicFolder[];
|
||||
pickLibrary: (id: 'all' | string) => void;
|
||||
visibleLibraryConfigs: SidebarItemConfig[];
|
||||
libraryItemsForReorder: SidebarItemConfig[];
|
||||
visibleSystemConfigs: SidebarItemConfig[];
|
||||
systemItemsForReorder: SidebarItemConfig[];
|
||||
playlistsExpanded: boolean;
|
||||
setPlaylistsExpanded: (v: boolean) => void;
|
||||
playlists: { id: string; name: string }[];
|
||||
playlistsLoading: boolean;
|
||||
newReleasesUnreadCount: number;
|
||||
navDnd: NavDndState | null;
|
||||
navDndRowClass: (section: 'library' | 'system', sectionIdx: number) => string;
|
||||
handleNavRowPointerDown: (e: React.PointerEvent, section: 'library' | 'system', sectionIdx: number) => void;
|
||||
navDndRowClass: (section: 'library' | 'system', id: string) => string;
|
||||
handleNavRowPointerDown: (e: React.PointerEvent, section: 'library' | 'system', id: string) => void;
|
||||
isPlaying: boolean;
|
||||
hasNowPlayingTrack: boolean;
|
||||
nowPlayingAtTop: boolean;
|
||||
@@ -60,8 +58,8 @@ export default function SidebarNavBody(props: Props) {
|
||||
isCollapsed, showLibraryPicker, filterId, selectedFolderName,
|
||||
libraryDropdownOpen, setLibraryDropdownOpen, dropdownRect, libraryTriggerRef,
|
||||
musicFolders, pickLibrary,
|
||||
visibleLibraryConfigs, libraryItemsForReorder,
|
||||
visibleSystemConfigs, systemItemsForReorder,
|
||||
visibleLibraryConfigs,
|
||||
visibleSystemConfigs,
|
||||
playlistsExpanded, setPlaylistsExpanded, playlists, playlistsLoading,
|
||||
newReleasesUnreadCount, navDnd, navDndRowClass, handleNavRowPointerDown,
|
||||
isPlaying, hasNowPlayingTrack, nowPlayingAtTop, hasOfflineContent,
|
||||
@@ -107,16 +105,15 @@ export default function SidebarNavBody(props: Props) {
|
||||
{visibleLibraryConfigs.map(cfg => {
|
||||
const item = ALL_NAV_ITEMS[cfg.id];
|
||||
if (!item) return null;
|
||||
const sectionIdx = libraryItemsForReorder.findIndex(x => x.id === cfg.id);
|
||||
const dndRow = !isCollapsed && sectionIdx >= 0;
|
||||
const rowClass = dndRow ? navDndRowClass('library', sectionIdx) : undefined;
|
||||
const dndRow = !isCollapsed;
|
||||
const rowClass = dndRow ? navDndRowClass('library', cfg.id) : undefined;
|
||||
const dndProps = dndRow
|
||||
? {
|
||||
'data-sidebar-nav-dnd-row': '',
|
||||
'data-sidebar-section': 'library' as const,
|
||||
'data-sidebar-idx': String(sectionIdx),
|
||||
'data-sidebar-id': cfg.id,
|
||||
onPointerDown: (e: React.PointerEvent) =>
|
||||
handleNavRowPointerDown(e, 'library', sectionIdx),
|
||||
handleNavRowPointerDown(e, 'library', cfg.id),
|
||||
}
|
||||
: {};
|
||||
|
||||
@@ -220,16 +217,15 @@ export default function SidebarNavBody(props: Props) {
|
||||
{visibleSystemConfigs.map(cfg => {
|
||||
const item = ALL_NAV_ITEMS[cfg.id];
|
||||
if (!item) return null;
|
||||
const sectionIdx = systemItemsForReorder.findIndex(x => x.id === cfg.id);
|
||||
const dndRow = !isCollapsed && sectionIdx >= 0;
|
||||
const rowClass = dndRow ? navDndRowClass('system', sectionIdx) : undefined;
|
||||
const dndRow = !isCollapsed;
|
||||
const rowClass = dndRow ? navDndRowClass('system', cfg.id) : undefined;
|
||||
const dndProps = dndRow
|
||||
? {
|
||||
'data-sidebar-nav-dnd-row': '',
|
||||
'data-sidebar-section': 'system' as const,
|
||||
'data-sidebar-idx': String(sectionIdx),
|
||||
'data-sidebar-id': cfg.id,
|
||||
onPointerDown: (e: React.PointerEvent) =>
|
||||
handleNavRowPointerDown(e, 'system', sectionIdx),
|
||||
handleNavRowPointerDown(e, 'system', cfg.id),
|
||||
}
|
||||
: {};
|
||||
|
||||
|
||||
@@ -1,9 +1,7 @@
|
||||
import React, { useCallback, useEffect, useRef, useState } from 'react';
|
||||
import type { SidebarItemConfig } from '../store/sidebarStore';
|
||||
import {
|
||||
applySidebarDropReorder,
|
||||
getLibraryItemsForReorder,
|
||||
getSystemItemsForReorder,
|
||||
applySidebarReorderById,
|
||||
isSidebarNavItemUserHideable,
|
||||
type SidebarNavDropTarget,
|
||||
} from '../utils/componentHelpers/sidebarNavReorder';
|
||||
@@ -15,13 +13,12 @@ import {
|
||||
|
||||
interface NavDndState {
|
||||
section: 'library' | 'system';
|
||||
fromIdx: number;
|
||||
draggedId: string;
|
||||
}
|
||||
|
||||
interface Args {
|
||||
isCollapsed: boolean;
|
||||
sidebarItemsRef: React.MutableRefObject<SidebarItemConfig[]>;
|
||||
randomNavModeRef: React.MutableRefObject<'hub' | 'separate'>;
|
||||
setSidebarItems: (items: SidebarItemConfig[]) => void;
|
||||
}
|
||||
|
||||
@@ -30,12 +27,12 @@ interface Result {
|
||||
navDropTarget: SidebarNavDropTarget | null;
|
||||
navDndTrashHint: { x: number; y: number } | null;
|
||||
suppressNavClickRef: React.MutableRefObject<boolean>;
|
||||
handleNavRowPointerDown: (e: React.PointerEvent, section: 'library' | 'system', sectionIdx: number) => void;
|
||||
navDndRowClass: (section: 'library' | 'system', sectionIdx: number) => string;
|
||||
handleNavRowPointerDown: (e: React.PointerEvent, section: 'library' | 'system', id: string) => void;
|
||||
navDndRowClass: (section: 'library' | 'system', id: string) => string;
|
||||
}
|
||||
|
||||
export function useSidebarNavDnd({
|
||||
isCollapsed, sidebarItemsRef, randomNavModeRef, setSidebarItems,
|
||||
isCollapsed, sidebarItemsRef, setSidebarItems,
|
||||
}: Args): Result {
|
||||
const [navDnd, setNavDnd] = useState<NavDndState | null>(null);
|
||||
const [navDropTarget, setNavDropTarget] = useState<SidebarNavDropTarget | null>(null);
|
||||
@@ -69,13 +66,13 @@ export function useSidebarNavDnd({
|
||||
const section = row.dataset.sidebarSection as 'library' | 'system' | undefined;
|
||||
if (section !== navDnd.section) continue;
|
||||
const rect = row.getBoundingClientRect();
|
||||
const idx = Number(row.dataset.sidebarIdx);
|
||||
if (Number.isNaN(idx)) continue;
|
||||
const id = row.dataset.sidebarId;
|
||||
if (!id) continue;
|
||||
if (clientY < rect.top + rect.height / 2) {
|
||||
target = { idx, before: true, section };
|
||||
target = { id, before: true, section };
|
||||
break;
|
||||
}
|
||||
target = { idx, before: false, section };
|
||||
target = { id, before: false, section };
|
||||
}
|
||||
navDropTargetRef.current = target;
|
||||
setNavDropTarget(target);
|
||||
@@ -99,11 +96,7 @@ export function useSidebarNavDnd({
|
||||
|
||||
const { x, y } = lastPointerDuringNavDndRef.current;
|
||||
if (isPointerOutsideAsideSidebar(x, y)) {
|
||||
const sectionItems =
|
||||
currentDnd.section === 'library'
|
||||
? getLibraryItemsForReorder(sidebarItemsRef.current, randomNavModeRef.current)
|
||||
: getSystemItemsForReorder(sidebarItemsRef.current);
|
||||
const id = sectionItems[currentDnd.fromIdx]?.id;
|
||||
const id = currentDnd.draggedId;
|
||||
if (id && isSidebarNavItemUserHideable(id)) {
|
||||
const nextItems: SidebarItemConfig[] = sidebarItemsRef.current.map(i =>
|
||||
i.id === id ? { ...i, visible: false } : i,
|
||||
@@ -114,12 +107,11 @@ export function useSidebarNavDnd({
|
||||
return;
|
||||
}
|
||||
|
||||
const next = applySidebarDropReorder(
|
||||
const next = applySidebarReorderById(
|
||||
sidebarItemsRef.current,
|
||||
currentDnd.section,
|
||||
currentDnd.fromIdx,
|
||||
currentDnd.draggedId,
|
||||
drop,
|
||||
randomNavModeRef.current,
|
||||
);
|
||||
if (next) {
|
||||
setSidebarItems(next);
|
||||
@@ -132,11 +124,7 @@ export function useSidebarNavDnd({
|
||||
lastPointerDuringNavDndRef.current = { x: e.clientX, y: e.clientY };
|
||||
|
||||
const outside = isPointerOutsideAsideSidebar(e.clientX, e.clientY);
|
||||
const sectionItems =
|
||||
navDnd.section === 'library'
|
||||
? getLibraryItemsForReorder(sidebarItemsRef.current, randomNavModeRef.current)
|
||||
: getSystemItemsForReorder(sidebarItemsRef.current);
|
||||
const draggedId = sectionItems[navDnd.fromIdx]?.id;
|
||||
const draggedId = navDnd.draggedId;
|
||||
const canTrash = Boolean(draggedId && isSidebarNavItemUserHideable(draggedId));
|
||||
if (outside && canTrash) {
|
||||
setNavDndTrashHint({ x: e.clientX, y: e.clientY });
|
||||
@@ -174,10 +162,10 @@ export function useSidebarNavDnd({
|
||||
document.body.style.userSelect = '';
|
||||
setNavDndTrashHint(null);
|
||||
};
|
||||
}, [navDnd, setSidebarItems, sidebarItemsRef, randomNavModeRef]);
|
||||
}, [navDnd, setSidebarItems, sidebarItemsRef]);
|
||||
|
||||
const handleNavRowPointerDown = useCallback(
|
||||
(e: React.PointerEvent, section: 'library' | 'system', sectionIdx: number) => {
|
||||
(e: React.PointerEvent, section: 'library' | 'system', id: string) => {
|
||||
if (isCollapsed || navDnd) return;
|
||||
if (e.pointerType === 'mouse' && e.button !== 0) return;
|
||||
|
||||
@@ -217,9 +205,9 @@ export function useSidebarNavDnd({
|
||||
cleanupEarly();
|
||||
window.getSelection()?.removeAllRanges();
|
||||
lastPointerDuringNavDndRef.current = { x: sx, y: sy };
|
||||
setNavDnd({ section, fromIdx: sectionIdx });
|
||||
navDropTargetRef.current = { idx: sectionIdx, before: true, section };
|
||||
setNavDropTarget({ idx: sectionIdx, before: true, section });
|
||||
setNavDnd({ section, draggedId: id });
|
||||
navDropTargetRef.current = { id, before: true, section };
|
||||
setNavDropTarget({ id, before: true, section });
|
||||
try {
|
||||
(e.currentTarget as HTMLElement).setPointerCapture(pid);
|
||||
} catch {
|
||||
@@ -236,14 +224,14 @@ export function useSidebarNavDnd({
|
||||
);
|
||||
|
||||
const navDndRowClass = useCallback(
|
||||
(section: 'library' | 'system', sectionIdx: number) => {
|
||||
const dragging = navDnd?.section === section && navDnd.fromIdx === sectionIdx;
|
||||
(section: 'library' | 'system', id: string) => {
|
||||
const dragging = navDnd?.section === section && navDnd.draggedId === id;
|
||||
let drop = '';
|
||||
if (
|
||||
navDnd &&
|
||||
navDropTarget?.section === section &&
|
||||
navDropTarget.idx === sectionIdx &&
|
||||
!(navDnd.section === section && navDnd.fromIdx === sectionIdx)
|
||||
navDropTarget.id === id &&
|
||||
!(navDnd.section === section && navDnd.draggedId === id)
|
||||
) {
|
||||
drop = navDropTarget.before
|
||||
? 'sidebar-nav-dnd-row--drop-before'
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { resolveStartRoute } from './sidebarNavReorder';
|
||||
import { applySidebarReorderById, resolveStartRoute } from './sidebarNavReorder';
|
||||
import { DEFAULT_SIDEBAR_ITEMS, type SidebarItemConfig } from '../../store/sidebarStore';
|
||||
|
||||
const hide = (items: SidebarItemConfig[], ...ids: string[]): SidebarItemConfig[] =>
|
||||
@@ -8,6 +8,102 @@ const hide = (items: SidebarItemConfig[], ...ids: string[]): SidebarItemConfig[]
|
||||
const only = (...ids: string[]): SidebarItemConfig[] =>
|
||||
DEFAULT_SIDEBAR_ITEMS.map(i => ({ ...i, visible: ids.includes(i.id) }));
|
||||
|
||||
const ids = (items: SidebarItemConfig[]): string[] => items.map(i => i.id);
|
||||
|
||||
describe('applySidebarReorderById', () => {
|
||||
it('moves a library item to before the target by id', () => {
|
||||
// Default order has artists(8) then composers, genres(10). Move genres up.
|
||||
const next = applySidebarReorderById(
|
||||
DEFAULT_SIDEBAR_ITEMS, 'library', 'genres',
|
||||
{ id: 'artists', before: true, section: 'library' },
|
||||
);
|
||||
expect(next).not.toBeNull();
|
||||
const order = ids(next!);
|
||||
expect(order.indexOf('genres')).toBe(order.indexOf('artists') - 1);
|
||||
expect(next!.length).toBe(DEFAULT_SIDEBAR_ITEMS.length);
|
||||
});
|
||||
|
||||
it('moves a library item to after the target by id', () => {
|
||||
const next = applySidebarReorderById(
|
||||
DEFAULT_SIDEBAR_ITEMS, 'library', 'artists',
|
||||
{ id: 'genres', before: false, section: 'library' },
|
||||
);
|
||||
const order = ids(next!);
|
||||
expect(order.indexOf('artists')).toBe(order.indexOf('genres') + 1);
|
||||
});
|
||||
|
||||
it('reorders correctly even when a hidden/gated item sits between the rows', () => {
|
||||
// The #1164 class: luckyMix is present in the stored list but hidden from
|
||||
// render. An index-based reorder desynced here; the id-based one must not.
|
||||
// Move genres to just before artists; luckyMix must keep its absolute slot.
|
||||
const luckyMixIdx = DEFAULT_SIDEBAR_ITEMS.findIndex(i => i.id === 'luckyMix');
|
||||
const next = applySidebarReorderById(
|
||||
DEFAULT_SIDEBAR_ITEMS, 'library', 'genres',
|
||||
{ id: 'artists', before: true, section: 'library' },
|
||||
);
|
||||
const order = ids(next!);
|
||||
expect(order.indexOf('genres')).toBe(order.indexOf('artists') - 1);
|
||||
expect(order.indexOf('luckyMix')).toBe(luckyMixIdx); // untouched anchor
|
||||
});
|
||||
|
||||
it('returns null on an unknown dragged id (defensive guard)', () => {
|
||||
expect(applySidebarReorderById(
|
||||
DEFAULT_SIDEBAR_ITEMS, 'library', 'does-not-exist',
|
||||
{ id: 'artists', before: true, section: 'library' },
|
||||
)).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null on an unknown target id (defensive guard)', () => {
|
||||
expect(applySidebarReorderById(
|
||||
DEFAULT_SIDEBAR_ITEMS, 'library', 'genres',
|
||||
{ id: 'does-not-exist', before: true, section: 'library' },
|
||||
)).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null when the section does not match the drop target', () => {
|
||||
expect(applySidebarReorderById(
|
||||
DEFAULT_SIDEBAR_ITEMS, 'library', 'genres',
|
||||
{ id: 'statistics', before: true, section: 'system' },
|
||||
)).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null when an id does not belong to the claimed section', () => {
|
||||
// 'statistics' is a system item — cannot be reordered as a library row.
|
||||
expect(applySidebarReorderById(
|
||||
DEFAULT_SIDEBAR_ITEMS, 'system', 'statistics',
|
||||
{ id: 'genres', before: true, section: 'system' },
|
||||
)).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null for a conserved (non-reorderable) id', () => {
|
||||
expect(applySidebarReorderById(
|
||||
DEFAULT_SIDEBAR_ITEMS, 'library', 'losslessAlbums',
|
||||
{ id: 'genres', before: true, section: 'library' },
|
||||
)).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null on a no-op drop (onto itself or its own edge)', () => {
|
||||
expect(applySidebarReorderById(
|
||||
DEFAULT_SIDEBAR_ITEMS, 'library', 'artists',
|
||||
{ id: 'artists', before: true, section: 'library' },
|
||||
)).toBeNull();
|
||||
// Dropping artists before composers — the row right after it — changes nothing.
|
||||
expect(applySidebarReorderById(
|
||||
DEFAULT_SIDEBAR_ITEMS, 'library', 'artists',
|
||||
{ id: 'composers', before: true, section: 'library' },
|
||||
)).toBeNull();
|
||||
});
|
||||
|
||||
it('does not mutate the input array', () => {
|
||||
const snapshot = ids(DEFAULT_SIDEBAR_ITEMS);
|
||||
applySidebarReorderById(
|
||||
DEFAULT_SIDEBAR_ITEMS, 'library', 'genres',
|
||||
{ id: 'artists', before: true, section: 'library' },
|
||||
);
|
||||
expect(ids(DEFAULT_SIDEBAR_ITEMS)).toEqual(snapshot);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveStartRoute', () => {
|
||||
it('falls back to the first visible library item when Mainstage is hidden', () => {
|
||||
const items = hide(DEFAULT_SIDEBAR_ITEMS, 'mainstage');
|
||||
|
||||
@@ -4,11 +4,18 @@ import { CONSERVED_SIDEBAR_NAV_IDS, type SidebarItemConfig } from '../../store/s
|
||||
export type SidebarNavSection = 'library' | 'system';
|
||||
|
||||
export type SidebarNavDropTarget = {
|
||||
idx: number;
|
||||
/** Stable id of the row the cursor is over — never a positional index. */
|
||||
id: string;
|
||||
before: boolean;
|
||||
section: SidebarNavSection;
|
||||
};
|
||||
|
||||
/** True when `id` is a real, non-conserved nav item that lives in `section`. */
|
||||
function itemBelongsToSection(id: string, section: SidebarNavSection): boolean {
|
||||
if (CONSERVED_SIDEBAR_NAV_IDS.has(id)) return false;
|
||||
return ALL_NAV_ITEMS[id]?.section === section;
|
||||
}
|
||||
|
||||
export function getLibraryItemsForReorder(
|
||||
items: SidebarItemConfig[],
|
||||
randomNavMode: 'hub' | 'separate',
|
||||
@@ -57,37 +64,44 @@ export function isSidebarNavItemUserHideable(id: string): boolean {
|
||||
}
|
||||
|
||||
/**
|
||||
* Reorders one sidebar section (library or system) like the Settings customizer.
|
||||
* Returns a new `items` array, or null if nothing changes.
|
||||
* Reorders one sidebar section by **stable item id**, not by positional index.
|
||||
*
|
||||
* The dragged row and the drop target are identified by id, and the move is
|
||||
* applied directly to the canonical full `items` array. This deliberately has
|
||||
* no shared index space with whatever filter decides which rows are *shown*:
|
||||
* a render filter (visibility, luckyMix availability, randomNavMode gating, any
|
||||
* future gate) can never desync the reorder, because indices are resolved here
|
||||
* from ids against the same array that is mutated. Hidden/gated items keep their
|
||||
* absolute slot and are never anchors.
|
||||
*
|
||||
* Returns a new `items` array, or `null` when nothing should change — unknown
|
||||
* id, cross-section drop, dropping onto self, or a no-op edge (defensive guard
|
||||
* against any payload the canonical list does not contain).
|
||||
*/
|
||||
export function applySidebarDropReorder(
|
||||
export function applySidebarReorderById(
|
||||
allItems: SidebarItemConfig[],
|
||||
section: SidebarNavSection,
|
||||
fromIdx: number,
|
||||
draggedId: string,
|
||||
target: SidebarNavDropTarget | null,
|
||||
randomNavMode: 'hub' | 'separate',
|
||||
): SidebarItemConfig[] | null {
|
||||
if (!target || target.section !== section) return null;
|
||||
const targetId = target.id;
|
||||
if (draggedId === targetId) return null;
|
||||
|
||||
const sectionItems =
|
||||
section === 'library'
|
||||
? [...getLibraryItemsForReorder(allItems, randomNavMode)]
|
||||
: [...getSystemItemsForReorder(allItems)];
|
||||
// Guard: both ids must be real, non-conserved items that belong to `section`.
|
||||
if (!itemBelongsToSection(draggedId, section)) return null;
|
||||
if (!itemBelongsToSection(targetId, section)) return null;
|
||||
|
||||
const insertBefore = target.before ? target.idx : target.idx + 1;
|
||||
if (insertBefore === fromIdx || insertBefore === fromIdx + 1) return null;
|
||||
const fromIdx = allItems.findIndex(c => c.id === draggedId);
|
||||
if (fromIdx < 0) return null;
|
||||
|
||||
const [moved] = sectionItems.splice(fromIdx, 1);
|
||||
sectionItems.splice(insertBefore > fromIdx ? insertBefore - 1 : insertBefore, 0, moved);
|
||||
|
||||
const visibleIds = new Set(sectionItems.map(c => c.id));
|
||||
const next = [...allItems];
|
||||
const positions = next
|
||||
.map((cfg, i) => ({ cfg, i }))
|
||||
.filter(({ cfg }) => visibleIds.has(cfg.id))
|
||||
.map(({ i }) => i);
|
||||
positions.forEach((pos, i) => {
|
||||
next[pos] = sectionItems[i];
|
||||
});
|
||||
const [moved] = next.splice(fromIdx, 1);
|
||||
const anchor = next.findIndex(c => c.id === targetId);
|
||||
if (anchor < 0) return null;
|
||||
next.splice(target.before ? anchor : anchor + 1, 0, moved);
|
||||
|
||||
// No-op if the resulting order is identical (e.g. dropped on its own edge).
|
||||
if (next.every((c, i) => c.id === allItems[i].id)) return null;
|
||||
return next;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user