Skip to content

Commit 23073ff

Browse files
deodorhunterclaude
deodorhunter
and
claude
authored
fix: upload review fixes (#424)
* fix(upload): a11y — configurable heading level in drag-drop, aria-required on file inputs - it-upload-drag-drop: replaces hardcoded <h5> with a configurable heading-level prop (default h3, styled via class="h5" so visual size is unchanged). Follows the unsafeStatic/staticHtml pattern used by it-card and it-notification. Adds DRAG_DROP_HEADING_LEVELS const + DragDropHeadingLevel type to types.ts. Updates SCSS to apply the same margin/color rules to all heading tags, not only h5. Exposes heading-level select control in Storybook. - it-upload-drag-drop + it-upload-avatar: adds aria-required="true" to the visible <input type="file"> when required is set, so AT users hear the required state on the interactive element they actually tab to. The hidden proxy input (needed for FormControl native validation) is unchanged. Tests: 10 new assertions in upload-drag-drop.test.ts and upload-avatar.test.ts; all 106 tests pass on Chromium, Firefox, WebKit. Closes #77 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: review fixes, better a11y * chore: changeset --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 2e97096 commit 23073ff

9 files changed

Lines changed: 175 additions & 57 deletions

File tree

‎.changeset/cool-bears-stand.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@italia/upload': minor
3+
---
4+
5+
Resolve a11y issues for it-upload-avatar and it-upload-drag-drop in form validation partecipation

‎packages/upload/src/it-upload-avatar.ts‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,20 @@ export class ItUploadAvatar extends FormControl {
7474
if (changedProperties.has('src') && !this._currentSrc) {
7575
this._currentSrc = this.src;
7676
}
77-
}
78-
79-
override updated(changedProperties: Map<string | number | symbol, unknown>) {
80-
super.updated?.(changedProperties);
81-
// Re-evaluate validity whenever the form value changes.
82-
if (changedProperties.has('_currentFile') || changedProperties.has('_currentSrc')) {
83-
this.handleValidationMessages();
77+
// Keep the displayed validation message in sync with the value here (pre-render) rather
78+
// than in updated(), so picking/clearing a file refreshes it without scheduling a second
79+
// render — avoids Lit's "scheduled an update after an update completed" warning. The native
80+
// `?required` attribute still drives checkValidity()/the constraint bubble. Guarded by
81+
// `hasUpdated` because `inputElement` only exists once the first render has produced the DOM.
82+
if (
83+
this.hasUpdated &&
84+
!this.customValidation &&
85+
(changedProperties.has('_currentFile') || changedProperties.has('_currentSrc'))
86+
) {
87+
const hasValue = Boolean(this._currentFile || this._currentSrc);
88+
const message = this.required && !hasValue ? this.$t('validityRequired') : '';
89+
this.inputElement?.setCustomValidity(message);
90+
this.validationMessage = message;
8491
}
8592
}
8693

@@ -110,14 +117,14 @@ export class ItUploadAvatar extends FormControl {
110117
);
111118
}
112119

113-
// Forward wrapper clicks to the hidden file input.
114-
// If the click already originated from the label (which has a `for` association), skip to
115-
116120
override render() {
117121
const labelText = this.$t('upload_avatar_label');
122+
const fileInputLabel = this.required ? labelText : labelText;
118123
const overlayText = this.overlayLabel ?? this.$t('upload_avatar_overlay_label');
119-
const proxyValue = this._currentSrc || this._currentFile?.name || '';
124+
// A pre-filled `src` or a freshly selected file both satisfy `required`.
125+
const hasValue = Boolean(this._currentFile || this._currentSrc);
120126
const isInvalid = this.formControlController.submittedOnce && this.validationMessage.length > 0;
127+
const feedbackId = `invalid-feedback-${this._id}`;
121128

122129
return html`
123130
<div class="avatar-upload-wrapper size-${this.size}">
@@ -132,11 +139,15 @@ export class ItUploadAvatar extends FormControl {
132139
<div class="upload-avatar-container">
133140
<input
134141
type="file"
135-
class="upload-avatar"
142+
class="upload-avatar it-form__control"
136143
id="${this._id!}"
137144
accept="${this.accept}"
138145
?disabled="${this.disabled}"
139-
aria-label="${labelText}"
146+
?required="${this.required && !hasValue}"
147+
aria-label="${fileInputLabel}"
148+
aria-required="${this.required ? 'true' : nothing}"
149+
aria-invalid="${isInvalid ? 'true' : 'false'}"
150+
aria-describedby="${ifDefined(isInvalid ? feedbackId : undefined)}"
140151
@change="${this._handleFileChange}"
141152
/>
142153
<label part="overlay-label" for="${this._id!}" class="it-upload-avatar-label-container" aria-hidden="true">
@@ -150,17 +161,8 @@ export class ItUploadAvatar extends FormControl {
150161
</div>
151162
</div>
152163
153-
<!-- Hidden proxy input: drives native required/validity checking via FormControl base class -->
154-
<input
155-
type="text"
156-
class="it-form__control"
157-
.value="${proxyValue}"
158-
?required="${this.required}"
159-
tabindex="-1"
160-
aria-hidden="true"
161-
/>
162-
163164
<div
165+
id="${feedbackId}"
164166
class="invalid-feedback form-feedback form-text just-validate-error-label"
165167
role="alert"
166168
?hidden=${!isInvalid}

‎packages/upload/src/it-upload-drag-drop.ts‎

Lines changed: 44 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@
22
import { FormControl, FormControlController } from '@italia/globals';
33
import { registerTranslation } from '@italia/i18n';
44
import { html, nothing } from 'lit';
5+
import { html as staticHtml, unsafeStatic } from 'lit/static-html.js';
56
import { customElement, property, query, state } from 'lit/decorators.js';
67
import { ifDefined } from 'lit/directives/if-defined.js';
78

89
import it from './locales/it.js';
910
import en from './locales/en.js';
1011
import styles from './upload-drag-drop.scss';
1112
import { formatSize } from './utils.js';
13+
import { type DragDropHeadingLevel, DRAG_DROP_HEADING_LEVELS } from './types.js';
1214

1315
registerTranslation(it);
1416
registerTranslation(en);
@@ -41,6 +43,10 @@ export class ItUploadDragDrop extends FormControl {
4143
@property({ type: String })
4244
illustration?: string;
4345

46+
/** Heading level for the title element. Defaults to 'h5'; visual size stays at h5. */
47+
@property({ type: String, attribute: 'heading-level' })
48+
headingLevel: DragDropHeadingLevel = 'h5';
49+
4450
/** The currently selected File (null until file is chosen/dropped). */
4551
@state()
4652
private _currentFile: File | null = null;
@@ -65,6 +71,10 @@ export class ItUploadDragDrop extends FormControl {
6571

6672
private _autoStarted = false;
6773

74+
protected getHeadingLevel(): DragDropHeadingLevel {
75+
return DRAG_DROP_HEADING_LEVELS.includes(this.headingLevel) ? this.headingLevel : 'h5';
76+
}
77+
6878
private get _formClass(): string {
6979
return this.composeClass(
7080
'upload-dragdrop',
@@ -243,6 +253,11 @@ export class ItUploadDragDrop extends FormControl {
243253

244254
// Title: slot or default per state
245255
const titleText = hasFile ? this._fileName : this.$t('upload_dd_title');
256+
const fileInputLabel = this.required ? this.$t('upload_label') : this.$t('upload_label');
257+
const canSelect = this._state === 'idle' || this._state === 'dragover';
258+
const isInvalid = this.formControlController.submittedOnce && this.validationMessage.length > 0;
259+
const feedbackId = `invalid-feedback-${this._id}`;
260+
const headingTag = unsafeStatic(this.getHeadingLevel());
246261

247262
return html`
248263
<div class="upload-dragdrop-text">
@@ -254,28 +269,35 @@ export class ItUploadDragDrop extends FormControl {
254269
</p>
255270
`
256271
: nothing}
257-
<h5>
272+
${staticHtml`<${headingTag} class="h5">
258273
<slot name="title">${titleText}</slot>
259-
</h5>
260-
${this._state === 'idle' || this._state === 'dragover'
274+
</${headingTag}>`} ${canSelect
261275
? html`
262276
<p>
263277
<slot name="description">${this.$t('upload_dd_description')}</slot>
264278
</p>
265-
<p>
266-
<input
267-
type="file"
268-
class="upload-dragdrop-input"
269-
id="${this._id!}"
270-
accept="${ifDefined(this.accept)}"
271-
?disabled="${this.disabled}"
272-
aria-label="${this.$t('upload_label')}"
273-
@change="${this._onFileInputChange}"
274-
/>
275-
<label for="${this._id!}">${this.$t('upload_dd_select')}</label>
276-
</p>
277279
`
278280
: nothing}
281+
<!-- The file input is the form-control validity participant, so it must exist in every
282+
state for the FormControl base class to read its validity. It is the real focusable
283+
control native required/validity now anchors to (no hidden proxy). The select affordance
284+
(label) and this wrapper are hidden via CSS in loading/success states. -->
285+
<p class="upload-dragdrop-select-wrap">
286+
<input
287+
type="file"
288+
class="upload-dragdrop-input it-form__control"
289+
id="${this._id!}"
290+
accept="${ifDefined(this.accept)}"
291+
?disabled="${this.disabled}"
292+
?required="${this.required && !this._currentFile}"
293+
aria-label="${fileInputLabel}"
294+
aria-required="${this.required ? 'true' : nothing}"
295+
aria-invalid="${isInvalid ? 'true' : 'false'}"
296+
aria-describedby="${ifDefined(isInvalid ? feedbackId : undefined)}"
297+
@change="${this._onFileInputChange}"
298+
/>
299+
${canSelect ? html`<label for="${this._id!}">${this.$t('upload_dd_select')}</label>` : nothing}
300+
</p>
279301
${this._state === 'loading' ? html`<p>${this.$t('upload_dd_status_loading')}</p>` : nothing}
280302
${this._state === 'success' ? html`<p>${this.$t('upload_dd_status_success')}</p>` : nothing}
281303
</div>
@@ -285,6 +307,7 @@ export class ItUploadDragDrop extends FormControl {
285307
override render() {
286308
const statusText = this._getStatusText();
287309
const isInvalid = this.formControlController.submittedOnce && this.validationMessage.length > 0;
310+
const feedbackId = `invalid-feedback-${this._id}`;
288311

289312
return html`
290313
<div
@@ -319,17 +342,12 @@ export class ItUploadDragDrop extends FormControl {
319342
<div role="status" aria-live="polite" aria-atomic="true" class="visually-hidden">${statusText}</div>
320343
</div>
321344
322-
<!-- Hidden proxy input: drives native required/validity checking via FormControl base class -->
323-
<input
324-
type="text"
325-
class="it-form__control"
326-
.value="${this._currentFile?.name || ''}"
327-
?required="${this.required}"
328-
tabindex="-1"
329-
aria-hidden="true"
330-
/>
331-
332-
<div class="invalid-feedback form-feedback form-text just-validate-error-label" role="alert">
345+
<div
346+
id="${feedbackId}"
347+
class="invalid-feedback form-feedback form-text just-validate-error-label"
348+
role="alert"
349+
?hidden=${!isInvalid}
350+
>
333351
${isInvalid ? this.validationMessage : nothing}
334352
</div>
335353
`;

‎packages/upload/src/types.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
export type UploadFileStatus = 'loading' | 'success' | 'error';
22

3+
export const DRAG_DROP_HEADING_LEVELS = ['h2', 'h3', 'h4', 'h5', 'h6'] as const;
4+
export type DragDropHeadingLevel = (typeof DRAG_DROP_HEADING_LEVELS)[number];
5+
36
export type UploadVariant = 'default' | 'gallery';
47

58
export interface UploadFile {

‎packages/upload/src/upload-avatar.scss‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,6 @@
1717
display: inline-block;
1818
}
1919

20-
// Hidden proxy input used by FormControl for required/validity — not visible to users.
21-
.it-form__control {
22-
display: none;
23-
}
24-
2520
// Override BSI's z-index: -1 which paints the input behind the stacking context,
2621
// making even the native focus ring invisible.
2722
// On mobile, BSI sets the input to 0.1px × 0.1px — iOS Safari will not open the

‎packages/upload/src/upload-drag-drop.scss‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,20 @@
1414
display: inline-block;
1515
}
1616

17-
// Hidden proxy input used by FormControl for required/validity — not visible to users.
18-
.it-form__control {
17+
// The file input is the form-control validity participant and must stay in the DOM in every
18+
// state, but the "select from device" affordance only makes sense before a file is chosen.
19+
// Hide its wrapper once a file is loading/uploaded (the input is no longer required there).
20+
.upload-dragdrop.loading .upload-dragdrop-select-wrap,
21+
.upload-dragdrop.success .upload-dragdrop-select-wrap {
1922
display: none;
2023
}
2124

25+
// Counter BSI's form-validation-state mixin which sets .invalid-feedback { display: none }.
26+
// We control visibility via the HTML `hidden` attribute (?hidden in Lit) instead.
27+
.invalid-feedback:not([hidden]) {
28+
display: block;
29+
}
30+
2231
// The progress component is positioned inside .upload-dragdrop-loading.
2332
// it-progress as a host element naturally fills the container via its own :host { display: block }
2433
// We size it via the wrapper to avoid crossing the it-progress shadow boundary.
@@ -51,3 +60,12 @@
5160
align-items: center;
5261
gap: var(--#{$prefix}spacing-xxs, 8px);
5362
}
63+
64+
// BSI's form-input-upload only styles `h5` inside .upload-dragdrop-text.
65+
// When heading-level is set to anything else, apply the same base rules.
66+
.upload-dragdrop-text {
67+
:is(h1, h2, h3, h4, h6) {
68+
margin: 0;
69+
color: var(--#{$prefix}color-text-base);
70+
}
71+
}

‎packages/upload/stories/it-upload.stories.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { ifDefined } from 'lit/directives/if-defined.js';
66
import { StoryFormControlMethodAndProps } from '@italia/globals';
77

88
import type { AvatarSize } from '@italia/avatar';
9-
import type { UploadVariant } from '../src/types.js';
9+
import { DRAG_DROP_HEADING_LEVELS, type DragDropHeadingLevel, type UploadVariant } from '../src/types.js';
1010
import type { ItUpload } from '../src/it-upload.js';
1111
import type { ItUploadDragDrop } from '../src/it-upload-drag-drop.js';
1212
import i18nIT from '../src/locales/it.js';
@@ -191,6 +191,7 @@ interface UploadDragDropProps {
191191
illustration: string;
192192
disabled: boolean;
193193
required: boolean;
194+
'heading-level': DragDropHeadingLevel;
194195
}
195196

196197
const renderUpload = (params: Partial<UploadProps>) => html`
@@ -225,6 +226,7 @@ const renderUploadDragDrop = (params: Partial<UploadDragDropProps>) => html`
225226
accept="${ifDefined(params.accept || undefined)}"
226227
name="${ifDefined(params.name || undefined)}"
227228
illustration="${ifDefined(params.illustration || undefined)}"
229+
heading-level="${ifDefined(params['heading-level'] || undefined)}"
228230
?disabled="${params.disabled}"
229231
?required="${params.required}"
230232
></it-upload-drag-drop>
@@ -369,6 +371,7 @@ export const EsempioInterattivoDragDrop: Story = {
369371
illustration: '',
370372
disabled: false,
371373
required: false,
374+
'heading-level': 'h3',
372375
},
373376
argTypes: {
374377
accept: {
@@ -379,6 +382,12 @@ export const EsempioInterattivoDragDrop: Story = {
379382
description: 'URL illustrazione personalizzata. Se omesso viene usata quella predefinita.',
380383
control: 'text',
381384
},
385+
'heading-level': {
386+
description: 'Livello del titolo semantico nella gerarchia della pagina. La dimensione visiva rimane h5.',
387+
control: { type: 'select' },
388+
options: DRAG_DROP_HEADING_LEVELS,
389+
table: { defaultValue: { summary: 'h5' } },
390+
},
382391
disabled: {
383392
description: 'Disabilita il componente.',
384393
control: 'boolean',

‎packages/upload/test/upload-avatar.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,4 +297,20 @@ describe('<it-upload-avatar>', () => {
297297
expect(feedback!.hasAttribute('aria-hidden')).to.be.false;
298298
expect(feedback!.textContent?.trim()).to.equal('');
299299
});
300+
301+
// ── aria-required on visible file input ───────────────────────────────────────
302+
303+
it('file input has aria-required="true" when required', async () => {
304+
const el = await fixture<ItUploadAvatar>(html`<it-upload-avatar name="avatar" required></it-upload-avatar>`);
305+
await el.updateComplete;
306+
const input = el.shadowRoot!.querySelector('input[type="file"]')!;
307+
expect(input.getAttribute('aria-required')).to.equal('true');
308+
});
309+
310+
it('file input has no aria-required when not required', async () => {
311+
const el = await fixture<ItUploadAvatar>(html`<it-upload-avatar name="avatar"></it-upload-avatar>`);
312+
await el.updateComplete;
313+
const input = el.shadowRoot!.querySelector('input[type="file"]')!;
314+
expect(input.hasAttribute('aria-required')).to.be.false;
315+
});
300316
});

0 commit comments

Comments
 (0)