Improve tag editor add-field UX and fix custom tag writes

- Fix custom tag keys being silently dropped by TagLib — unknown keys are now uppercased before write since TagLib requires all-caps Vorbis field names
- Add real-time validation for invalid characters (= and non-ASCII) in custom tag keys
- Show 'Field already exists' error on Enter/+ attempt rather than silently doing nothing
- Resolve typed tag labels to canonical keys (e.g. 'Album Sort' → albumSort) to prevent duplicate entries under a different key
- Atomic duplicate check in hook functional updater prevents race conditions from rapid adds
- Scroll to and focus newly added field after it is inserted
- Consolidate FIELD_LABEL_OVERRIDES, FIELD_TYPE_OVERRIDES, and PICARD_EXTRAS into a single TAG_CONFIG object; extract resolveTagKey helper
This commit is contained in:
ChrisScott9456
2026-07-11 17:28:49 -04:00
parent cde2b85148
commit c594545658
6 changed files with 226 additions and 237 deletions
@@ -1,16 +1,19 @@
import { Autocomplete } from '@mantine/core';
import { closeAllModals } from '@mantine/modals';
import { useRef } from 'react';
import { useRef, useState } from 'react';
import { RiAddLine } from 'react-icons/ri';
import { useTranslation } from 'react-i18next';
import { useMetadataEditor } from '../hooks/use-metadata-editor';
import { KNOWN_TAG_MAP, resolveTagKey } from '../utils/known-tags';
import { ArtworkPanel } from './artwork-panel';
import styles from './song-edit-modal.module.css';
import { TagFieldRow } from './tag-field-row';
import { ActionIcon } from '/@/shared/components/action-icon/action-icon';
import { Button } from '/@/shared/components/button/button';
import { Checkbox } from '/@/shared/components/checkbox/checkbox';
import { Group } from '/@/shared/components/group/group';
import { Select } from '/@/shared/components/select/select';
import { Spinner } from '/@/shared/components/spinner/spinner';
import { Stack } from '/@/shared/components/stack/stack';
import { Table } from '/@/shared/components/table/table';
@@ -21,27 +24,55 @@ import { Song } from '/@/shared/types/domain-types';
export const SongEditModal = ({ songs }: { songs: Song[] }) => {
const { t } = useTranslation();
const tableContainerRef = useRef<HTMLDivElement>(null);
const skipNextEnterRef = useRef(false);
const skipNextOnChangeResetRef = useRef(false);
const [addFieldInput, setAddFieldInput] = useState('');
const [duplicateAttempted, setDuplicateAttempted] = useState(false);
// Opens the metadata editor modal that displays a table of editable fields for the songs, as well as an artwork panel for changing the songs' artwork
const editor = useMetadataEditor({
browser: window.api.browser,
songs,
utils: window.api.utils,
});
// After adding the field, scrolls to and focuses the new field's row in the table.
const handleAddField = (key: null | string) => {
editor.handleAddField(key);
if (!key) return;
const trimmedInput = addFieldInput.trim();
const customKeyError =
trimmedInput && !KNOWN_TAG_MAP.has(trimmedInput)
? trimmedInput.includes('=')
? "Tag key cannot contain '='"
: /[^\x00-\x7F]/.test(trimmedInput)
? 'Tag key must use ASCII characters only'
: null
: null;
const resolvedInputKey = trimmedInput ? resolveTagKey(trimmedInput) : '';
const duplicateError =
trimmedInput && !customKeyError && editor.sortedFieldEntries.some(([k]) => k === resolvedInputKey)
? 'Field already exists'
: null;
const fieldError = customKeyError ?? (duplicateAttempted ? duplicateError : null);
const handleAddField = (key: string): boolean => {
const trimmed = key.trim();
if (!trimmed) return false;
if (!KNOWN_TAG_MAP.has(trimmed) && (trimmed.includes('=') || /[^\x00-\x7F]/.test(trimmed))) return false;
const normalizedKey = resolveTagKey(trimmed);
if (editor.sortedFieldEntries.some(([k]) => k === normalizedKey)) {
setDuplicateAttempted(true);
return false;
}
setDuplicateAttempted(false);
editor.handleAddField(normalizedKey);
setAddFieldInput('');
// Scroll to focus on the newly added field
requestAnimationFrame(() => {
const row = tableContainerRef.current?.querySelector<HTMLElement>(
`[data-field-key="${key}"]`,
`[data-field-key="${normalizedKey}"]`,
);
row?.scrollIntoView({ block: 'nearest' });
row?.querySelector<HTMLElement>('input, textarea')?.focus();
});
return true;
};
// While loading, shows a spinner
@@ -86,15 +117,47 @@ export const SongEditModal = ({ songs }: { songs: Song[] }) => {
{editor.readWarning}
</Text>
)}
{editor.availableToAdd.length > 0 && (
<Select
clearable
data={editor.availableToAdd}
onChange={handleAddField}
placeholder={t('page.itemDetail.addField', 'Add field…')}
value={null}
/>
)}
<Autocomplete
data={editor.availableToAdd}
error={fieldError}
onChange={(v) => {
setAddFieldInput(v);
if (skipNextOnChangeResetRef.current) {
skipNextOnChangeResetRef.current = false;
} else {
setDuplicateAttempted(false);
}
}}
onKeyDown={(e) => {
if (e.key === 'Enter') {
if (skipNextEnterRef.current) {
skipNextEnterRef.current = false;
return;
}
handleAddField(addFieldInput);
}
}}
onOptionSubmit={(value) => {
skipNextEnterRef.current = true;
skipNextOnChangeResetRef.current = true;
const added = handleAddField(value);
if (added) {
queueMicrotask(() => setAddFieldInput(''));
}
}}
placeholder={t('page.itemDetail.addField', 'Add field…')}
rightSection={
<ActionIcon
disabled={!!fieldError}
onClick={() => handleAddField(addFieldInput)}
variant="filled"
>
<RiAddLine size={16} />
</ActionIcon>
}
rightSectionPointerEvents="all"
value={addFieldInput}
/>
<div className={styles.tableScroller} ref={tableContainerRef}>
<Table
classNames={{ td: styles.tableCell, th: styles.tableHeader }}
@@ -6,11 +6,11 @@ import type {
} from '/@/shared/types/tag-editor';
import { closeAllModals } from '@mantine/modals';
import { useCallback, useEffect, useMemo, useState } from 'react';
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { useTranslation } from 'react-i18next';
import { FIELD_PRIORITY, KNOWN_TAG_MAP, KNOWN_TAGS, type KnownTag } from '../utils/known-tags';
import { base64ToBytes, filterTagSummary, formatBatchFileErrors } from '../utils/utils';
import { base64ToBytes, formatBatchFileErrors } from '../utils/utils';
import { controller } from '/@/renderer/api/controller';
import { useCurrentServer } from '/@/renderer/store';
@@ -145,7 +145,7 @@ export const useMetadataEditor = ({ browser, songs: songsProp, utils }: UseMetad
);
}
setTagSummary(filterTagSummary(batchResult.tagSummary));
setTagSummary(batchResult.tagSummary);
if (
batchResult.artworkKind === 'common' &&
@@ -184,10 +184,21 @@ export const useMetadataEditor = ({ browser, songs: songsProp, utils }: UseMetad
setRemovedKeys((prev) => new Set(prev).add(key));
}, []);
// Refs so the functional updater inside handleAddField can see latest values without needing them in the dependency array.
const tagSummaryRef = useRef(tagSummary);
tagSummaryRef.current = tagSummary;
const removedKeysRef = useRef(removedKeys);
removedKeysRef.current = removedKeys;
/** Adds `key` to `editedFields` with an empty value and un-marks it from removal. */
const handleAddField = useCallback((key: null | string) => {
if (!key) return;
setEditedFields((prev) => ({ ...prev, [key]: '' }));
setEditedFields((prev) => {
const wasRemoved = removedKeysRef.current.has(key);
const alreadyVisible = (key in tagSummaryRef.current || key in prev) && !wasRemoved;
if (alreadyVisible) return prev;
return { ...prev, [key]: '' };
});
setRemovedKeys((prev) => {
const next = new Set(prev);
next.delete(key);
@@ -1,6 +1,4 @@
import type { EditorFieldKey } from '/@/shared/types/tag-editor';
import { EDITOR_FIELD_KEYS } from '/@/shared/types/tag-editor';
import { PROPERTIES } from 'taglib-wasm';
/** Tags pinned to the top of the editor table (most frequently edited). */
export const FIELD_PRIORITY: readonly string[] = [
@@ -13,42 +11,6 @@ export const FIELD_PRIORITY: readonly string[] = [
'date',
];
/** Human-readable labels where auto-generated text is awkward. */
const FIELD_LABEL_OVERRIDES: Partial<Record<EditorFieldKey, string>> = {
acoustidId: 'AcoustID',
albumArtist: 'Album Artist',
albumArtistSort: 'Album Artist Sort',
albumSort: 'Album Sort',
artistSort: 'Artist Sort',
catalogNumber: 'Catalog Number',
composerSort: 'Composer Sort',
discNumber: 'Disc Number',
musicbrainzArtistId: 'MusicBrainz Artist ID',
musicbrainzReleaseArtistId: 'MusicBrainz Album Artist ID',
musicbrainzReleaseGroupId: 'MusicBrainz Release Group ID',
musicbrainzReleaseId: 'MusicBrainz Album ID',
musicbrainzReleaseTrackId: 'MusicBrainz Release Track ID',
musicbrainzTrackId: 'MusicBrainz Track ID',
musicbrainzWorkId: 'MusicBrainz Work ID',
originalAlbum: 'Original Album',
originalArtist: 'Original Artist',
originalDate: 'Original Date',
remixedBy: 'Remixer',
titleSort: 'Title Sort',
totalDiscs: 'Total Discs',
totalTracks: 'Total Tracks',
trackNumber: 'Track Number',
};
/** Input widget overrides (taglib metadata type is not always specific enough). */
const FIELD_TYPE_OVERRIDES: Partial<
Record<EditorFieldKey, 'boolean' | 'number' | 'string' | 'textarea'>
> = {
bpm: 'number',
comment: 'textarea',
lyrics: 'textarea',
};
export interface KnownTag {
key: string;
label: string;
@@ -58,20 +20,81 @@ export interface KnownTag {
/** Which form control to render for a tag row. */
export type TagFieldType = 'boolean' | 'number' | 'string' | 'textarea';
/**
* Per-key overrides and extras. Keys present in taglib-wasm PROPERTIES get their
* label/type overridden; keys absent from PROPERTIES are appended as extra entries.
*/
const TAG_CONFIG: Record<string, { label?: string; type?: TagFieldType }> = {
acoustidId: { label: 'AcoustID' },
albumArtist: { label: 'Album Artist' },
albumArtistSort: { label: 'Album Artist Sort' },
albumSort: { label: 'Album Sort' },
artistSort: { label: 'Artist Sort' },
catalogNumber: { label: 'Catalog Number' },
composerSort: { label: 'Composer Sort' },
discNumber: { label: 'Disc Number' },
musicbrainzArtistId: { label: 'MusicBrainz Artist ID' },
musicbrainzReleaseArtistId: { label: 'MusicBrainz Album Artist ID' },
musicbrainzReleaseGroupId: { label: 'MusicBrainz Release Group ID' },
musicbrainzReleaseId: { label: 'MusicBrainz Album ID' },
musicbrainzReleaseTrackId: { label: 'MusicBrainz Release Track ID' },
musicbrainzTrackId: { label: 'MusicBrainz Track ID' },
musicbrainzWorkId: { label: 'MusicBrainz Work ID' },
originalAlbum: { label: 'Original Album' },
originalArtist: { label: 'Original Artist' },
originalDate: { label: 'Original Date' },
remixedBy: { label: 'Remixer' },
titleSort: { label: 'Title Sort' },
totalDiscs: { label: 'Total Discs' },
totalTracks: { label: 'Total Tracks' },
trackNumber: { label: 'Track Number' },
acoustidFingerprint: { type: 'textarea' },
bpm: { type: 'number' },
comment: { type: 'textarea' },
lyrics: { type: 'textarea' },
// extras not in PROPERTIES (common MusicBrainz Picard tags)
ARTISTS: { label: 'Artists', type: 'string' },
ORIGINALYEAR: { label: 'Original Year', type: 'number' },
RELEASECOUNTRY: { label: 'Release Country', type: 'string' },
RELEASESTATUS: { label: 'Release Status', type: 'string' },
RELEASETYPE: { label: 'Release Type', type: 'string' },
};
const humanizeKey = (key: string): string =>
key
.replace(/([A-Z])/g, ' $1')
.replace(/^./, (c) => c.toUpperCase())
.trim();
const resolveFieldType = (key: string): TagFieldType =>
FIELD_TYPE_OVERRIDES[key as EditorFieldKey] ?? 'string';
/** Field definitions used to render the tag editor table and "Add field" dropdown. */
export const KNOWN_TAGS: KnownTag[] = EDITOR_FIELD_KEYS.map((key) => ({
key,
label: FIELD_LABEL_OVERRIDES[key] ?? humanizeKey(key),
type: resolveFieldType(key),
}));
/** Field definitions derived from taglib-wasm's PROPERTIES plus any extras in TAG_CONFIG. */
export const KNOWN_TAGS: KnownTag[] = [
...Object.entries(PROPERTIES).map(([key, prop]) => {
const cfg = TAG_CONFIG[key];
return {
key,
label: cfg?.label ?? humanizeKey(key),
type: cfg?.type ?? (prop.type as TagFieldType),
};
}),
...Object.entries(TAG_CONFIG)
.filter(([key]) => !(key in PROPERTIES))
.map(([key, cfg]) => ({
key,
label: cfg.label ?? humanizeKey(key),
type: cfg.type ?? ('string' as TagFieldType),
})),
];
export const KNOWN_TAG_MAP = new Map(KNOWN_TAGS.map((t) => [t.key, t]));
/**
* Resolves a user-typed string to a canonical tag key.
* Matches by label first (e.g. "Album Sort" → "albumSort"), then falls back to
* the raw input. Unknown keys are uppercased so TagLib writes them as valid
* Vorbis comment field names.
*/
export const resolveTagKey = (input: string): string => {
const byLabel = KNOWN_TAGS.find((t) => t.label.toLowerCase() === input.toLowerCase());
const key = byLabel?.key ?? input;
return KNOWN_TAG_MAP.has(key) ? key : key.toUpperCase();
};
@@ -1,7 +1,5 @@
import type { BatchFileError } from '/@/shared/types/tag-editor';
import { KNOWN_TAG_MAP } from './known-tags';
export const base64ToBytes = (base64: string): Uint8Array => {
const binary = atob(base64);
const bytes = new Uint8Array(binary.length);
@@ -17,13 +15,3 @@ export const formatBatchFileErrors = (failed: BatchFileError[], summary: string)
const suffix = failed.length > 3 ? '…' : '';
return `${summary} ${details}${suffix}`;
};
export const filterTagSummary = (
tagSummary: Record<string, null | string>,
): Record<string, null | string> => {
const filtered: Record<string, null | string> = {};
for (const [k, v] of Object.entries(tagSummary)) {
if (KNOWN_TAG_MAP.has(k)) filtered[k] = v;
}
return filtered;
};