@@ -1665,7 +1669,7 @@ export class PlaylistDetails
>
Add to Queue
@@ -1720,7 +1724,7 @@ export class PlaylistDetails
>
Add to Playlist
-
+
New Playlist
diff --git a/frontend/src/components/playlist-view/playlist-view.ts b/frontend/src/components/playlist-view/playlist-view.ts
index 4e7aa62..8ffe1cd 100644
--- a/frontend/src/components/playlist-view/playlist-view.ts
+++ b/frontend/src/components/playlist-view/playlist-view.ts
@@ -37,6 +37,10 @@ import { ViewLifecycleMixin } from '@utils/view-lifecycle';
import { FavoritesController } from '@store/controllers/favorites-controller';
import '@components/duplicate-tracks-dialog/duplicate-tracks-dialog.js';
import type { DuplicateTracksDialog } from '@components/duplicate-tracks-dialog/duplicate-tracks-dialog.js';
+import {
+ ICON_NEW,
+ ICON_PLAYLIST,
+} from '@utils/icon-language';
const SCROLL_DEBOUNCE_MS = 100;
@@ -1496,7 +1500,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
@dragleave=${this.onNewButtonDragLeave}
@drop=${this.onNewButtonDrop}
>
-
Queue is empty
@@ -1875,7 +1880,7 @@ export class QueuePanel
>
Add to Playlist
Add to Queue
@@ -1521,7 +1525,7 @@ export class SmartPlaylistDetails
>
Add to Playlist
this.onContextMenuAction('add-to-queue')}
@mouseenter=${() => this.ctxMenu.closePlaylistSubmenu()}
>
-
+
Add to Queue
-
+
Add to Playlist
diff --git a/frontend/src/icons/names.txt b/frontend/src/icons/names.txt
index 81ce440..309e42b 100644
--- a/frontend/src/icons/names.txt
+++ b/frontend/src/icons/names.txt
@@ -24,6 +24,7 @@ solid/arrows-rotate
solid/arrow-up-short-wide
solid/backward-step
solid/bars
+solid/bars-staggered
regular/bookmark
solid/bookmark
solid/box-open
diff --git a/frontend/src/utils/icon-language.ts b/frontend/src/utils/icon-language.ts
new file mode 100644
index 0000000..28ffc94
--- /dev/null
+++ b/frontend/src/utils/icon-language.ts
@@ -0,0 +1,100 @@
+/**
+ * What each icon in this app means, once.
+ *
+ * The set was a mix: `plus` meant "add to the queue", "add to a
+ * playlist", "make a new playlist" and "you do not own this" — the
+ * first two *adjacent in the same context menu* — while `list` meant
+ * the queue, the Playlists destination, and (in `queue-panel` alone)
+ * adding to the queue. Two icons carrying seven meanings between them
+ * is not a vocabulary, and a user cannot learn one that says four
+ * things.
+ *
+ * The rule these are chosen by: **an icon names the noun it acts on,
+ * not the verb.** "Add to queue" and "add to playlist" are the same
+ * verb on different nouns, so the noun is what has to differ — which is
+ * also why adding to a playlist wears the Playlists destination's own
+ * icon rather than a generic plus. `plus` survives for exactly the one
+ * thing it is unambiguous about, making something that did not exist.
+ *
+ * Import these rather than writing a name inline. A literal string is
+ * how the last set drifted, and nothing catches it: a wrong-but-real
+ * icon renders perfectly.
+ */
+
+/** Start playing this now. */
+export const ICON_PLAY = 'play';
+
+/** Start playing this now, in a shuffled order. */
+export const ICON_SHUFFLE = 'shuffle';
+
+/**
+ * The queue, and putting something into it.
+ *
+ * One glyph for the noun and the action, so the button that opens the
+ * queue and the menu item that adds to it are visibly the same subject.
+ * The queue used to wear `list`, which is the Playlists destination.
+ */
+export const ICON_QUEUE = 'bars-staggered';
+
+/** Put this next in the queue rather than at the end. */
+export const ICON_PLAY_NEXT = 'forward-step';
+
+/**
+ * A playlist, and adding something to one.
+ *
+ * The same icon as the Playlists destination in the sidebar, which is
+ * the point: the menu item says where the thing is going.
+ */
+export const ICON_PLAYLIST = 'list';
+
+/**
+ * Make a new thing that did not exist — a playlist, a rule, a library.
+ *
+ * This is the only meaning `plus` keeps. It used to carry four.
+ */
+export const ICON_NEW = 'plus';
+
+/**
+ * The request ("want") toggle, as an outline/solid pair.
+ *
+ * Two states of one control have to read as each other's opposite,
+ * which a plus and a bookmark do not. The pair was already in the app
+ * and already correct — `explore-album-details`'s "Want this" button
+ * has used it since it was written, and `favorites-controller` uses the
+ * same shape for `regular/heart` → `heart` — while the badge forty
+ * pixels away showed a plus for the same state.
+ *
+ * That is `utils/library-status.ts`'s fault one layer down: it made the
+ * two surfaces agree on *what wanting means* and left them disagreeing
+ * on what it looks like.
+ */
+export const ICON_CAN_REQUEST = 'regular/bookmark';
+export const ICON_REQUESTED = 'solid/bookmark';
+
+/**
+ * You have this.
+ *
+ * Deliberately not drawn on the common case — see the tracklist, where
+ * absence is what gets marked. This is for the places that answer the
+ * question directly, like the badge on a catalog card.
+ */
+export const ICON_IN_LIBRARY = 'check';
+
+/**
+ * Something is being fetched right now.
+ *
+ * Distinct from `ICON_REQUESTED`: a request may sit on the list
+ * forever without anything happening, which is exactly why the badge's
+ * "queued" state stopped being an hourglass.
+ */
+export const ICON_DOWNLOADING = 'download';
+
+/**
+ * Take this away.
+ *
+ * One icon for removing from a playlist, from the queue and from the
+ * library, because the difference that matters is stated in the words
+ * beside it and in the confirmation — "Remove from Library" says in its
+ * impact line that the files are not deleted.
+ */
+export const ICON_REMOVE = 'trash';
diff --git a/frontend/test/components/chrome.test.ts b/frontend/test/components/chrome.test.ts
index 4d11c12..1384145 100644
--- a/frontend/test/components/chrome.test.ts
+++ b/frontend/test/components/chrome.test.ts
@@ -19,6 +19,11 @@ import {
update,
visual,
} from '@test/support/render';
+import {
+ ICON_CAN_REQUEST,
+ ICON_IN_LIBRARY,
+ ICON_REQUESTED,
+} from '@utils/icon-language';
describe('', () => {
it('renders a testid per destination, which is how e2e navigates', async () => {
@@ -154,9 +159,20 @@ describe('', () => {
it('defaults to "not in library"', async () => {
const el = await fixture('library-status-indicator');
- expect(shadow(el, 'wa-icon')?.getAttribute('name')).toBe('plus');
+ expect(shadow(el, 'wa-icon')?.getAttribute('name')).toBe(ICON_CAN_REQUEST);
});
+ /**
+ * Named from the vocabulary rather than written out, or this test
+ * pins the glyphs *against* the table it is supposed to follow —
+ * which is what it did: it asserted `plus` for the un-owned state,
+ * the same glyph two adjacent menu items were using for two other
+ * meanings, and passing was the reason nobody looked.
+ *
+ * What is still worth asserting is that the three differ, which is
+ * the property the states need and the one the table cannot state
+ * about itself here.
+ */
it('uses a distinct glyph per state', async () => {
const glyphs: (string | null | undefined)[] = [];
@@ -166,7 +182,8 @@ describe('', () => {
glyphs.push(shadow(el, 'wa-icon')?.getAttribute('name'));
}
- expect(glyphs).toEqual(['check', 'bookmark', 'plus']);
+ expect(glyphs).toEqual([ICON_IN_LIBRARY, ICON_REQUESTED, ICON_CAN_REQUEST]);
+ expect(new Set(glyphs).size).toBe(3);
});
it('phrases its label around the entity it describes', async () => {
diff --git a/frontend/test/components/icon-language.test.ts b/frontend/test/components/icon-language.test.ts
new file mode 100644
index 0000000..1c14fe5
--- /dev/null
+++ b/frontend/test/components/icon-language.test.ts
@@ -0,0 +1,140 @@
+/**
+ * The icon vocabulary is one table, and nothing writes around it.
+ *
+ * A wrong-but-real icon name renders perfectly: no error, no fallback,
+ * no failing assertion anywhere. That is how `plus` came to mean "add
+ * to the queue", "add to a playlist", "make a new playlist" and "you do
+ * not own this" — the first two adjacent in the same context menu —
+ * while `list` meant the queue, the Playlists destination *and* adding
+ * to the queue.
+ *
+ * `src/icons/index.ts` catches a name that is not *bundled*. Nothing
+ * catches a name that is bundled and means something else, so this
+ * sweeps the source for the governed ones. It is the same shape as
+ * `TestNoDirectRuntimeEmits` and `TestNoWritesOnTheReadPool` in the
+ * backend, and exists for the same reason: the rule is about every call
+ * site, so checking one is checking nothing.
+ */
+import { describe, expect, it } from 'vitest';
+
+import { bundledIconNames } from '../../src/icons';
+import * as icons from '@utils/icon-language';
+
+/** Every component source, as text. */
+const SOURCES = import.meta.glob('../../src/**/*.ts', {
+ eager: true,
+ query: '?raw',
+ import: 'default',
+});
+
+/**
+ * The names that carry a meaning the table owns.
+ *
+ * Deliberately not every bundled name. `check` is `ICON_IN_LIBRARY`
+ * here and also the "Copied" confirmation in `job-log-view`, which is
+ * a different, perfectly good meaning — governing it would force a
+ * false rename. What belongs on this list is a name that was actually
+ * overloaded.
+ */
+const GOVERNED = [
+ 'plus',
+ 'list',
+ 'bookmark',
+ 'solid/bookmark',
+ 'regular/bookmark',
+ 'bars-staggered',
+];
+
+/** The one file allowed to say them, plus its own test. */
+const DEFINITION = /icon-language\.(ts|test\.ts)$/;
+
+describe('the icon vocabulary', () => {
+ /**
+ * A sweep over nothing passes. This is the assertion that makes the
+ * rest of the file mean something, and it is the first thing that
+ * breaks if the glob pattern stops matching after a move.
+ */
+ it('actually reads the source', () => {
+ const paths = Object.keys(SOURCES);
+
+ expect(paths.length).toBeGreaterThan(100);
+ expect(paths.some((p) => p.endsWith('/track-list.ts'))).toBe(true);
+ expect(SOURCES[paths[0]!]).toContain('import');
+ });
+
+ it.each(GOVERNED)('is not written around for %s', (name) => {
+ const offenders: string[] = [];
+
+ for (const [path, source] of Object.entries(SOURCES)) {
+ if (DEFINITION.test(path)) continue;
+
+ // Both spellings: an icon in a template, and an icon name in a
+ // data table (which is how the sidebar and bottom-nav carry
+ // theirs).
+ const literal = new RegExp(
+ `(name="${name}"|icon: '${name}'|name=\\$\\{[^}]*'${name}')`,
+ );
+
+ if (literal.test(source)) offenders.push(path);
+ }
+
+ expect(offenders).toEqual([]);
+ });
+
+ /**
+ * A meaning with no icon behind it is the state the badge's `queued`
+ * spent a year in — declared, styled, and produced by nothing.
+ */
+ it('gives every meaning a name', () => {
+ const values = Object.entries(icons).filter(([k]) => k.startsWith('ICON_'));
+
+ expect(values.length).toBeGreaterThan(0);
+
+ for (const [key, value] of values) {
+ expect(`${key}=${value}`).toMatch(/^ICON_[A-Z_]+=[a-z]+[a-z/-]*$/);
+ }
+ });
+
+ /**
+ * Every name in the table is a name the app actually ships.
+ *
+ * This is the loop the vocabulary closes. A name that is not bundled
+ * renders a circled question mark and reports itself to
+ * `__yjIconMisses` — at *runtime*, from a state something has to
+ * reach first. `bookmark-check` is Font Awesome **Pro**, and it was
+ * on `explore-artist-details`'s Follow button, drawn for every
+ * followed artist, invisible to `offline-icons.spec.ts` because no
+ * spec had ever followed one. Reaching the state is no longer how
+ * this is found.
+ */
+ it('names only icons that are bundled', () => {
+ const bundled = new Set(bundledIconNames());
+ const missing = Object.entries(icons)
+ .filter(([k]) => k.startsWith('ICON_'))
+ .filter(([, v]) => !bundled.has(v as string))
+ .map(([k, v]) => `${k} (${v})`);
+
+ expect(missing).toEqual([]);
+ });
+
+ /**
+ * The two states of the request toggle have to be the same glyph in
+ * two weights, or they do not read as each other's opposite — which
+ * is what a plus against a bookmark was.
+ */
+ it('makes the request toggle an outline/solid pair', () => {
+ expect(icons.ICON_CAN_REQUEST).toBe(`regular/${icons.ICON_REQUESTED.replace('solid/', '')}`);
+ });
+
+ /**
+ * The queue and the Playlists destination wore the same icon, and
+ * "add to queue" and "add to playlist" sat next to each other wearing
+ * a third same one. Whatever the table says, these three have to
+ * differ from each other.
+ */
+ it('keeps the queue, playlists and creating something apart', () => {
+ const three = [icons.ICON_QUEUE, icons.ICON_PLAYLIST, icons.ICON_NEW];
+
+ expect(new Set(three).size).toBe(3);
+ });
+});