From 510b24c2983d259fbdb89b02170ba31fa481ef4e Mon Sep 17 00:00:00 2001
From: Roland <roland@learningselfreliance.com>
Date: Thu, 30 Jul 2026 15:46:54 -0600
Subject: [PATCH] ui: name the directory fields, announce their help text, let
 long prefixes shrink

Greptile's three findings on #590, all confirmed real before fixing, and each one
verified red against the original code afterwards.

P1, the directory fields had no accessible names.  All five rendered their
visible label as a bare <label> with no htmlFor while passing no label prop, so
their inputs had a description ("/media/wrolpi/") and no name at all -- five
fields in one grid, holding similar relative paths, indistinguishable to a
screen reader.  The labels now belong to the inputs, InfoPopup content and all.
This predates PathInput; the old TextInput call sites had the same gap, and
moving to a component that supports labels is what made it fixable.

P1, description and error were rendered but never referenced.  aria-describedby
pointed only at the prefix, so the help text listing the variables a path
accepts, and the reason a path was rejected, were both invisible to assistive
technology.  Both are now in the description, and an errored field reports
aria-invalid.

P2 was worse than reported: at `flex: none` a long prefix took the entire row
and left the input eighteen pixels wide -- not "consumes the editable area" but
a field nobody can type in.  The media directory is whatever the API reports, so
its length was never ours to assume.  The prefix now shrinks and ellipsises at
half the control, with the full path on `title` and in the accessible
description.

The naming fault could not have been caught where the others were.  jsdom cannot
see layout, and the Settings page needs enough mocking that it had no test at
all -- which is why these five fields were uncovered when they broke.  The new
e2e spec intercepts the API and lets the real page render, so it measures the
names and the geometry a user actually gets.  Reverting one field to a bare
label fails it; a long prefix fails the browser spec at 18px.

962 jest, 15 browser component, 46 e2e, clean cache-busted build, tsc clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../e2e/settings/special-directories.cy.js    | 117 ++++++++++++++++++
 app/cypress/support/component.js              |   5 +-
 app/src/components/admin/Settings.js          |  80 ++++++------
 app/src/components/ui/Inputs.tsx              |  29 ++++-
 app/src/components/ui/ui-layout.cy.js         |  26 ++++
 app/src/components/ui/ui.css                  |  17 ++-
 app/src/components/ui/ui.test.js              |  36 ++++++
 7 files changed, 262 insertions(+), 48 deletions(-)
 create mode 100644 app/cypress/e2e/settings/special-directories.cy.js

diff --git a/app/cypress/e2e/settings/special-directories.cy.js b/app/cypress/e2e/settings/special-directories.cy.js
new file mode 100644
index 00000000..81da1294
--- /dev/null
+++ b/app/cypress/e2e/settings/special-directories.cy.js
@@ -0,0 +1,117 @@
+/*
+ * The five Special Directories fields on /admin/settings.
+ *
+ * These are the fields that shipped printing their own prefix over their own value, and the
+ * ones whose visible labels were never associated with their inputs.  Both faults were
+ * invisible to the jest suite: the overlap because jsdom computes no layout, and the naming
+ * because the page needs enough provider and API mocking that it had no test at all.
+ *
+ * Intercepting the API is not mocking the component -- the page really renders, so the
+ * accessible names and the real geometry are the ones a user gets.
+ */
+describe('Special Directories settings', () => {
+    const mockSettings = {
+        archive_destination: 'archive/%(domain_tag)s/%(domain)s',
+        download_manager_disabled: false,
+        download_manager_stopped: false,
+        download_on_startup: true,
+        download_timeout: 0,
+        download_wait: 20,
+        download_window_start: null,
+        download_window_end: null,
+        hotspot_device: 'wlan0',
+        hotspot_on_startup: true,
+        hotspot_password: 'wrolpi hotspot',
+        hotspot_ssid: 'WROLPi',
+        check_for_upgrades: true,
+        ignore_outdated_zims: false,
+        log_level: 'info',
+        map_destination: 'map',
+        nav_color: 'violet',
+        media_directory: '/media/wrolpi',
+        playlists_destination: 'playlists',
+        tags_directory: true,
+        throttle_on_startup: false,
+        version: '1.0.0',
+        videos_destination: 'videos/%(channel_tag)s/%(channel_name)s',
+        wrol_mode: false,
+        zims_destination: 'zims',
+        save_ffprobe_json: true,
+    };
+
+    const directories = [
+        ['Archive Directory', 'archive/%(domain_tag)s/%(domain)s'],
+        ['Videos Directory', 'videos/%(channel_tag)s/%(channel_name)s'],
+        ['Map Directory', 'map'],
+        ['Zims Directory', 'zims'],
+        ['Playlists Directory', 'playlists'],
+    ];
+
+    beforeEach(() => {
+        cy.intercept('GET', '/api/status', {
+            statusCode: 200,
+            body: {
+                version: '1.0.0',
+                flags: {},
+                cpu_percent: 10,
+                memory_percent: 30,
+                downloads: {
+                    pending: 0, recurring: 0, disabled: false, stopped: false,
+                    outside_download_window: false,
+                },
+            },
+        }).as('getStatus');
+        cy.intercept('GET', '/api/tags', {statusCode: 200, body: {tags: []}}).as('getTags');
+        cy.intercept('GET', '/api/events', {statusCode: 200, body: {events: []}}).as('getEvents');
+        cy.intercept('POST', '/api/search/suggestions', {
+            statusCode: 200,
+            body: {fileGroups: 0, zimsEstimates: [], channels: [], domains: []},
+        }).as('getSuggestions');
+        cy.intercept('GET', '/api/settings', {statusCode: 200, body: {...mockSettings}})
+            .as('getSettings');
+
+        cy.visit('/admin/settings');
+        cy.wait('@getSettings');
+    });
+
+    it('names every directory field, so they can be told apart', () => {
+        /*
+         * All five fields sit in one grid, hold similar-looking relative paths, and share the
+         * same prefix.  Without a name attached to the input, a screen reader user hears five
+         * fields described only as "/media/wrolpi/" and cannot tell which one saves videos.
+         */
+        directories.forEach(([label, value]) => {
+            cy.contains('label', label)
+                .invoke('attr', 'for')
+                .should('be.a', 'string')
+                .then((inputId) => {
+                    cy.get(`#${CSS.escape(inputId)}`)
+                        .should('have.value', value)
+                        .and('match', 'input');
+                });
+        });
+    });
+
+    it('shows the media directory beside each value, never over it', () => {
+        // The reported bug: `a/media/wrolpi/hain_tag)s/%(domain)s`.
+        cy.get('.wrolpi-path-input').should('have.length', directories.length);
+
+        cy.get('.wrolpi-path-input').each(($field) => {
+            const prefix = $field.find('.wrolpi-path-input-prefix')[0].getBoundingClientRect();
+            const input = $field.find('input')[0].getBoundingClientRect();
+            expect(prefix.right, 'prefix ends where the input begins')
+                .to.be.at.most(input.left + 0.5);
+            expect(input.width, 'input is wide enough to read and edit').to.be.greaterThan(60);
+        });
+    });
+
+    it('keeps the prefix out of the value the user edits and saves', () => {
+        cy.contains('label', 'Archive Directory').invoke('attr', 'for').then((inputId) => {
+            cy.get(`#${CSS.escape(inputId)}`)
+                .should('have.value', 'archive/%(domain_tag)s/%(domain)s')
+                .and('not.have.value', '/media/wrolpi/archive/%(domain_tag)s/%(domain)s');
+        });
+
+        cy.get('.wrolpi-path-input-prefix').first().should('have.text', '/media/wrolpi/');
+    });
+});
diff --git a/app/cypress/support/component.js b/app/cypress/support/component.js
index d27f01c5..dcb3f7b8 100644
--- a/app/cypress/support/component.js
+++ b/app/cypress/support/component.js
@@ -104,8 +104,9 @@ Cypress.Commands.add('shouldNotOverlap', {prevSubject: false}, (firstSelector, s
         cy.get(secondSelector).then(($second) => {
             const a = $first[0].getBoundingClientRect();
             const b = $second[0].getBoundingClientRect();
-            const overlaps = a.right > b.left + 0.5 && b.right > a.left + 0.5
-                && a.bottom > b.top + 0.5 && b.bottom > a.top + 0.5;
+            const overlapsHorizontally = (a.right > b.left + 0.5) && (b.right > a.left + 0.5);
+            const overlapsVertically = (a.bottom > b.top + 0.5) && (b.bottom > a.top + 0.5);
+            const overlaps = overlapsHorizontally && overlapsVertically;
             expect(
                 overlaps,
                 `${firstSelector} [${a.left}, ${a.right}] must not overlap `
diff --git a/app/src/components/admin/Settings.js b/app/src/components/admin/Settings.js
index 9a7e9919..98cf6f23 100644
--- a/app/src/components/admin/Settings.js
+++ b/app/src/components/admin/Settings.js
@@ -759,25 +759,25 @@ export function SettingsPage() {
 
                 <Grid>
                     <Grid.Col span={{base: 12, sm: 6}}>
-                        <label>
-                            Archive Directory
-                            <InfoPopup
-                                w={340}
-                                content={<>
-                                    <p>Variables:</p>
-                                    <ul>
-                                        <li><code>%(domain)s</code> - Domain name (e.g., "example.com")</li>
-                                        <li><code>%(domain_tag)s</code> - Tag name for the domain (empty if no
-                                            tag)
-                                        </li>
-                                        <li><code>%(tag)s</code> - Alias for the tag name</li>
-                                        <li><code>%(name)s</code> - Alias for the domain name</li>
-                                    </ul>
-                                </>}
-                                position='top'
-                            />
-                        </label>
                         <PathInput
+                            label={<>
+                                Archive Directory
+                                <InfoPopup
+                                    w={340}
+                                    content={<>
+                                        <p>Variables:</p>
+                                        <ul>
+                                            <li><code>%(domain)s</code> - Domain name (e.g., "example.com")</li>
+                                            <li><code>%(domain_tag)s</code> - Tag name for the domain (empty if no
+                                                tag)
+                                            </li>
+                                            <li><code>%(tag)s</code> - Alias for the tag name</li>
+                                            <li><code>%(name)s</code> - Alias for the domain name</li>
+                                        </ul>
+                                    </>}
+                                    position='top'
+                                />
+                            </>}
                             prefix={mediaDirectoryLabel}
                             value={state.archive_destination}
                             disabled={!editSpecialDirectories}
@@ -785,26 +785,26 @@ export function SettingsPage() {
                         />
                     </Grid.Col>
                     <Grid.Col span={{base: 12, sm: 6}}>
-                        <label>
-                            Videos Directory
-                            <InfoPopup
-                                w={340}
-                                content={<>
-                                    <p>Variables:</p>
-                                    <ul>
-                                        <li><code>%(channel_name)s</code> - Channel name</li>
-                                        <li><code>%(channel_tag)s</code> - Tag name for the channel (empty if no
-                                            tag)
-                                        </li>
-                                        <li><code>%(channel_domain)s</code> - Domain from the channel URL</li>
-                                        <li><code>%(tag)s</code> - Alias for the tag name</li>
-                                        <li><code>%(name)s</code> - Alias for the channel name</li>
-                                    </ul>
-                                </>}
-                                position='top'
-                            />
-                        </label>
                         <PathInput
+                            label={<>
+                                Videos Directory
+                                <InfoPopup
+                                    w={340}
+                                    content={<>
+                                        <p>Variables:</p>
+                                        <ul>
+                                            <li><code>%(channel_name)s</code> - Channel name</li>
+                                            <li><code>%(channel_tag)s</code> - Tag name for the channel (empty if no
+                                                tag)
+                                            </li>
+                                            <li><code>%(channel_domain)s</code> - Domain from the channel URL</li>
+                                            <li><code>%(tag)s</code> - Alias for the tag name</li>
+                                            <li><code>%(name)s</code> - Alias for the channel name</li>
+                                        </ul>
+                                    </>}
+                                    position='top'
+                                />
+                            </>}
                             prefix={mediaDirectoryLabel}
                             value={state.videos_destination}
                             disabled={!editSpecialDirectories}
@@ -812,8 +812,8 @@ export function SettingsPage() {
                         />
                     </Grid.Col>
                     <Grid.Col span={{base: 12, sm: 6}}>
-                        <label>Map Directory</label>
                         <PathInput
+                            label='Map Directory'
                             prefix={mediaDirectoryLabel}
                             value={state.map_destination}
                             disabled={!editSpecialDirectories}
@@ -821,8 +821,8 @@ export function SettingsPage() {
                         />
                     </Grid.Col>
                     <Grid.Col span={{base: 12, sm: 6}}>
-                        <label>Zims Directory</label>
                         <PathInput
+                            label='Zims Directory'
                             prefix={mediaDirectoryLabel}
                             value={state.zims_destination}
                             disabled={!editSpecialDirectories}
@@ -830,8 +830,8 @@ export function SettingsPage() {
                         />
                     </Grid.Col>
                     <Grid.Col span={{base: 12, sm: 6}}>
-                        <label>Playlists Directory</label>
                         <PathInput
+                            label='Playlists Directory'
                             prefix={mediaDirectoryLabel}
                             value={state.playlists_destination}
                             disabled={!editSpecialDirectories}
diff --git a/app/src/components/ui/Inputs.tsx b/app/src/components/ui/Inputs.tsx
index 8a3f5d74..ea0f9808 100644
--- a/app/src/components/ui/Inputs.tsx
+++ b/app/src/components/ui/Inputs.tsx
@@ -90,6 +90,20 @@ export const PathInput = forwardRef<HTMLInputElement, PathInputProps>((
     const generated = useId();
     const inputId = id ?? `${generated}-input`;
     const prefixId = `${generated}-prefix`;
+    const descriptionId = `${generated}-description`;
+    const errorId = `${generated}-error`;
+
+    /*
+     * Everything that explains the field, in reading order.  Drawing the description and the
+     * error on screen does not announce them -- an unreferenced element is invisible to a
+     * screen reader, which would leave a user hearing the label and the prefix but never the
+     * variables the path accepts, nor the reason a path was just rejected.
+     */
+    const describedBy = [
+        prefixId,
+        description ? descriptionId : null,
+        error ? errorId : null,
+    ].filter(Boolean).join(' ');
 
     return <div className={['wrolpi-path-input', className].filter(Boolean).join(' ')}>
         {label && <label className='wrolpi-path-input-label' htmlFor={inputId}>
@@ -100,20 +114,27 @@ export const PathInput = forwardRef<HTMLInputElement, PathInputProps>((
             {/*
               Described rather than hidden: a screen reader should hear which directory the
               typed path is relative to, since that is the whole point of showing it.
+
+              `title` carries the full path because the prefix truncates when it is long --
+              the media directory is whatever the API reports, and a prefix that refused to
+              shrink took the entire row and left the input 18px wide.
             */}
-            <span className='wrolpi-path-input-prefix' id={prefixId}>{prefix}</span>
+            <span className='wrolpi-path-input-prefix' id={prefixId} title={prefix}>{prefix}</span>
             <input
                 ref={ref}
                 id={inputId}
                 type='text'
                 required={required}
                 disabled={disabled}
-                aria-describedby={prefixId}
+                aria-describedby={describedBy}
+                aria-invalid={error ? true : undefined}
                 {...props}
             />
         </div>
-        {description && <div className='wrolpi-path-input-description'>{description}</div>}
-        {error && <div className='wrolpi-path-input-error'>{error}</div>}
+        {description && <div className='wrolpi-path-input-description' id={descriptionId}>
+            {description}
+        </div>}
+        {error && <div className='wrolpi-path-input-error' id={errorId}>{error}</div>}
     </div>
 });
 PathInput.displayName = 'PathInput';
diff --git a/app/src/components/ui/ui-layout.cy.js b/app/src/components/ui/ui-layout.cy.js
index 3e605237..9d3f2836 100644
--- a/app/src/components/ui/ui-layout.cy.js
+++ b/app/src/components/ui/ui-layout.cy.js
@@ -63,6 +63,32 @@ describe('PathInput lays its prefix beside the value, never over it', () => {
         });
     });
 
+    it('still leaves somewhere to type when the media directory is a long path', () => {
+        /*
+         * The prefix is whatever the API reports as the media directory, so its length is not
+         * ours to assume.  It is `/media/wrolpi` on a Pi, but a development box or an unusual
+         * Docker mount can report something far longer, and a prefix that refuses to shrink
+         * would take the whole row and leave the input at zero width -- a field that cannot
+         * be typed into at all.
+         */
+        cy.mountUI(
+            <div style={{width: 320}}>
+                <PathInput
+                    label='Archive Destination'
+                    prefix='/mnt/storage/external/wrolpi-media-library-volume/'
+                    defaultValue='archive/%(domain)s'
+                />
+            </div>,
+        );
+
+        cy.shouldNotOverlap('.wrolpi-path-input-prefix', '.wrolpi-path-input-control input');
+        cy.get('.wrolpi-path-input-control input').should(($input) =>
+            expect($input[0].clientWidth, 'input keeps a usable width').to.be.greaterThan(60));
+        // The control must not spill out of the column it was given either.
+        cy.get('.wrolpi-path-input-control').should(($control) =>
+            expect($control[0].getBoundingClientRect().width).to.be.at.most(320));
+    });
+
     it('detects the overlap that started this, so the check has teeth', () => {
         /*
          * The original defect, reproduced deliberately: Mantine turns leftSectionWidth into
diff --git a/app/src/components/ui/ui.css b/app/src/components/ui/ui.css
index 561453d2..3db1a4c0 100644
--- a/app/src/components/ui/ui.css
+++ b/app/src/components/ui/ui.css
@@ -429,11 +429,24 @@ html[data-theme="amber"] .wrolpi-path-input-control:focus-within {
     background: var(--bg);
 }
 
-/* Muted and on the header surface: it reads as context, not as something to edit. */
+/*
+ * Muted and on the header surface: it reads as context, not as something to edit.
+ *
+ * It shrinks and ellipsises rather than holding its full width.  The prefix is whatever the
+ * API reports as the media directory -- `/media/wrolpi` on a Pi, but a development box or an
+ * unusual Docker mount can report a much longer path -- and at `flex: none` a long one took
+ * the whole row and left the input 18px wide, a field nobody could type into.  Capping it at
+ * half the control keeps the editable part editable; the full path stays available through
+ * `title` and is read out in full by the accessible description either way.
+ */
 .wrolpi-path-input-prefix {
     display: flex;
     align-items: center;
-    flex: none;
+    flex: 0 1 auto;
+    min-width: 3ch;
+    max-width: 50%;
+    overflow: hidden;
+    text-overflow: ellipsis;
     padding: 0 6px 0 9px;
     background: var(--head);
     border-right: 1px solid var(--border);
diff --git a/app/src/components/ui/ui.test.js b/app/src/components/ui/ui.test.js
index e4bd104a..034c5842 100644
--- a/app/src/components/ui/ui.test.js
+++ b/app/src/components/ui/ui.test.js
@@ -343,6 +343,42 @@ describe('PathInput', () => {
         expect(screen.getByText('Not a directory')).toBeInTheDocument();
     });
 
+    it('reads out its help text as well as the prefix', () => {
+        /*
+         * Rendering the description on screen is not the same as announcing it.  Unless it is
+         * referenced, a screen reader user hears the label and the prefix and never learns
+         * what the field expects -- the help text is the part that explains the variables
+         * these paths are allowed to contain.
+         */
+        renderUI(<PathInput
+            prefix='/media/wrolpi/'
+            label='Archive Directory'
+            description='Accepts %(domain)s and %(tag)s.'
+            value=''
+            onChange={jest.fn()}
+        />);
+
+        expect(screen.getByLabelText(/Archive Directory/))
+            .toHaveAccessibleDescription('/media/wrolpi/ Accepts %(domain)s and %(tag)s.');
+    });
+
+    it('announces the reason a path was rejected', () => {
+        // The dashed border and red text carry the failure for sighted users; without the
+        // error in the accessible description nothing carries it for anyone else.
+        renderUI(<PathInput prefix='/media/wrolpi/' label='Zims Directory'
+                            error='Not a directory' value='' onChange={jest.fn()}/>);
+
+        const input = screen.getByLabelText(/Zims Directory/);
+        expect(input).toHaveAccessibleDescription(/Not a directory/);
+        expect(input).toBeInvalid();
+    });
+
+    it('is not marked invalid when there is nothing wrong with it', () => {
+        renderUI(<PathInput prefix='/media/wrolpi/' label='Fine' value='' onChange={jest.fn()}/>);
+
+        expect(screen.getByLabelText(/Fine/)).toBeValid();
+    });
+
     it('forwards a ref to the input, for callers that focus it', () => {
         const ref = React.createRef();
         renderUI(<PathInput ref={ref} prefix='/media/wrolpi/' label='Focusable'