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() { - + Archive Directory + +

Variables:

+
    +
  • %(domain)s - Domain name (e.g., "example.com")
    • +
  • %(domain_tag)s - Tag name for the domain (empty if no + tag) +
    • +
  • %(tag)s - Alias for the tag name
    • +
  • %(name)s - Alias for the domain name
    • +
+ } + position='top' + /> + } prefix={mediaDirectoryLabel} value={state.archive_destination} disabled={!editSpecialDirectories} @@ -785,26 +785,26 @@ export function SettingsPage() { /> - + Videos Directory + +

Variables:

+
    +
  • %(channel_name)s - Channel name
    • +
  • %(channel_tag)s - Tag name for the channel (empty if no + tag) +
    • +
  • %(channel_domain)s - Domain from the channel URL
    • +
  • %(tag)s - Alias for the tag name
    • +
  • %(name)s - Alias for the channel name
    • +
+ } + position='top' + /> + } prefix={mediaDirectoryLabel} value={state.videos_destination} disabled={!editSpecialDirectories} @@ -812,8 +812,8 @@ export function SettingsPage() { /> - - - (( 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
{label &&
- {description &&
{description}
} - {error &&
{error}
} + {description &&
+ {description} +
} + {error &&
{error}
} }); 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( +
+ +
, + ); + + 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(); + + 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(); + + 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(); + + expect(screen.getByLabelText(/Fine/)).toBeValid(); + }); + it('forwards a ref to the input, for callers that focus it', () => { const ref = React.createRef(); renderUI(