fix: set tabindex on the vaadin-upload-file element (#3068) (#3073)

Co-authored-by: Tomi Virkki <tomivirkki@users.noreply.github.com>
vaadin · Nov 17, 2021 · fdfbf61 · fdfbf61
1 parent 00b7087
commit fdfbf61
Show file tree

Hide file tree

Showing 10 changed files with 258 additions and 49 deletions.
diff --git a/dev/upload.html b/dev/upload.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html lang="en">
+  <head>
+    <meta charset="UTF-8" />
+    <meta http-equiv="X-UA-Compatible" content="IE=edge" />
+    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
+    <title>Upload</title>
+
+    <script type="module">
+      import '@vaadin/upload';
+
+      const upload = document.querySelector('vaadin-upload');
+      upload.files = [
+        { name: 'Annual Report.docx', complete: true },
+        {
+          name: 'Workflow.pdf',
+          progress: 60,
+          status: '19.7 MB: 60% (remaining time: 00:12:34)'
+        },
+        { name: 'Financials.xlsx', error: 'An error occurred' }
+      ];
+    </script>
+  </head>
+
+  <body>
+    <vaadin-upload target="/api/fileupload"></vaadin-upload>
+  </body>
+</html>
diff --git a/packages/upload/src/vaadin-upload-file.js b/packages/upload/src/vaadin-upload-file.js
@@ -68,10 +68,10 @@ class UploadFile extends ThemableMixin(PolymerElement) {
         }
       </style>
 
-      <li part="row" tabindex="0">
+      <div part="row">
         <div part="info">
-          <div part="done-icon" hidden$="[[!file.complete]]"></div>
-          <div part="warning-icon" hidden$="[[!file.error]]"></div>
+          <div part="done-icon" hidden$="[[!file.complete]]" aria-hidden="true"></div>
+          <div part="warning-icon" hidden$="[[!file.error]]" aria-hidden="true"></div>
 
           <div part="meta">
             <div part="name" id="name">[[file.name]]</div>
@@ -107,7 +107,7 @@ class UploadFile extends ThemableMixin(PolymerElement) {
             aria-describedby="name"
           ></button>
         </div>
-      </li>
+      </div>
 
       <vaadin-progress-bar
         part="progress"

diff --git a/packages/upload/src/vaadin-upload.js b/packages/upload/src/vaadin-upload.js
@@ -82,6 +82,7 @@ class Upload extends ElementMixin(ThemableMixin(PolymerElement)) {
         [part='file-list'] {
           padding: 0;
           margin: 0;
+          list-style-type: none;
         }
       </style>
 
@@ -103,7 +104,9 @@ class Upload extends ElementMixin(ThemableMixin(PolymerElement)) {
       <slot name="file-list">
         <ul id="fileList" part="file-list">
           <template is="dom-repeat" items="[[files]]" as="file">
-            <vaadin-upload-file file="[[file]]" i18n="[[i18n]]"></vaadin-upload-file>
+            <li>
+              <vaadin-upload-file tabindex="0" file="[[file]]" i18n="[[i18n]]"></vaadin-upload-file>
+            </li>
           </template>
         </ul>
       </slot>

diff --git a/packages/upload/test/a11y.test.js b/packages/upload/test/a11y.test.js
@@ -1,32 +1,13 @@
 import { expect } from '@esm-bundle/chai';
-import { fixtureSync, nextRender } from '@vaadin/testing-helpers';
+import { fixtureSync } from '@vaadin/testing-helpers';
 import '../vaadin-upload.js';
 import { createFile } from './common.js';
 
 const FAKE_FILE = createFile(100000, 'application/uknown');
 
 describe('a11y', () => {
-  describe('<vaadin-upload>', () => {
-    let uploadElement, fileList;
-
-    beforeEach(async () => {
-      uploadElement = fixtureSync(`<vaadin-upload></vaadin-upload>`);
-      uploadElement.files = [FAKE_FILE];
-
-      await nextRender();
-
-      fileList = uploadElement.shadowRoot.querySelector('[part=file-list]');
-    });
-
-    describe('file list', () => {
-      it('should be a <ul> element', () => {
-        expect(fileList).to.be.an.instanceOf(HTMLUListElement);
-      });
-    });
-  });
-
   describe('<vaadin-upload-file>', () => {
-    let uploadFileElement, i18n, button, name, row;
+    let uploadFileElement, i18n, button, name;
 
     beforeEach(() => {
       uploadFileElement = fixtureSync(`<vaadin-upload-file></vaadin-upload-file>`);
@@ -40,25 +21,14 @@ describe('a11y', () => {
       uploadFileElement.i18n = i18n;
       uploadFileElement.file = FAKE_FILE;
 
-      row = uploadFileElement.shadowRoot.querySelector('[part=row]');
       name = uploadFileElement.shadowRoot.querySelector('[part=name]');
     });
 
-    describe('row', () => {
-      it('should be a <li> element', () => {
-        expect(row).to.be.an.instanceOf(HTMLLIElement);
-      });
-    });
-
     describe('start button', () => {
       beforeEach(() => {
         button = uploadFileElement.shadowRoot.querySelector('[part=start-button]');
       });
 
-      it('should be a <button> element', () => {
-        expect(button).to.be.an.instanceOf(HTMLButtonElement);
-      });
-
       it('should have aria-describedby attribute', () => {
         expect(button.getAttribute('aria-describedby')).to.equal(name.id);
       });
@@ -73,10 +43,6 @@ describe('a11y', () => {
         button = uploadFileElement.shadowRoot.querySelector('[part=retry-button]');
       });
 
-      it('should be a <button> element', () => {
-        expect(button).to.be.an.instanceOf(HTMLButtonElement);
-      });
-
       it('should have aria-describedby attribute', () => {
         expect(button.getAttribute('aria-describedby')).to.equal(name.id);
       });
@@ -91,10 +57,6 @@ describe('a11y', () => {
         button = uploadFileElement.shadowRoot.querySelector('[part=remove-button]');
       });
 
-      it('should be a <button> element', () => {
-        expect(button).to.be.an.instanceOf(HTMLButtonElement);
-      });
-
       it('should have aria-describedby attribute', () => {
         expect(button.getAttribute('aria-describedby')).to.equal(name.id);
       });

diff --git a/packages/upload/test/dom/__snapshots__/vaadin-upload-file.test.snap.js b/packages/upload/test/dom/__snapshots__/vaadin-upload-file.test.snap.js
@@ -0,0 +1,79 @@
+/* @web/test-runner snapshot v1 */
+export const snapshots = {};
+
+snapshots["vaadin-upload-file shadow default"] = 
+`<div part="row">
+  <div part="info">
+    <div
+      aria-hidden="true"
+      hidden=""
+      part="done-icon"
+    >
+    </div>
+    <div
+      aria-hidden="true"
+      hidden=""
+      part="warning-icon"
+    >
+    </div>
+    <div part="meta">
+      <div
+        id="name"
+        part="name"
+      >
+        Workflow.pdf
+      </div>
+      <div
+        id="status"
+        part="status"
+      >
+        19.7 MB: 60% (remaining time: 00:12:34)
+      </div>
+      <div
+        hidden=""
+        id="error"
+        part="error"
+      >
+      </div>
+    </div>
+  </div>
+  <div part="commands">
+    <button
+      aria-describedby="name"
+      file-event="file-start"
+      hidden=""
+      part="start-button"
+      type="button"
+    >
+    </button>
+    <button
+      aria-describedby="name"
+      file-event="file-retry"
+      hidden=""
+      part="retry-button"
+      type="button"
+    >
+    </button>
+    <button
+      aria-describedby="name"
+      file-event="file-abort"
+      part="remove-button"
+      type="button"
+    >
+    </button>
+  </div>
+</div>
+<vaadin-progress-bar
+  aria-valuemax="1"
+  aria-valuemin="0"
+  aria-valuenow="0.6"
+  id="progress"
+  part="progress"
+  role="progressbar"
+  style="--vaadin-progress-value:0.6;"
+  value="0.6"
+>
+</vaadin-progress-bar>
+`;
+/* end snapshot vaadin-upload-file shadow default */
+
diff --git a/packages/upload/test/dom/__snapshots__/vaadin-upload.test.snap.js b/packages/upload/test/dom/__snapshots__/vaadin-upload.test.snap.js
@@ -0,0 +1,77 @@
+/* @web/test-runner snapshot v1 */
+export const snapshots = {};
+
+snapshots["vaadin-upload shadow default"] = 
+`<div part="primary-buttons">
+  <div id="addFiles">
+    <slot name="add-button">
+      <vaadin-button
+        id="addButton"
+        part="upload-button"
+        role="button"
+        tabindex="0"
+      >
+        Upload Files...
+      </vaadin-button>
+    </slot>
+  </div>
+  <div
+    id="dropLabelContainer"
+    part="drop-label"
+  >
+    <slot name="drop-label-icon">
+      <div part="drop-label-icon">
+      </div>
+    </slot>
+    <slot
+      id="dropLabel"
+      name="drop-label"
+    >
+      Drop files here
+    </slot>
+  </div>
+</div>
+<slot name="file-list">
+  <ul
+    id="fileList"
+    part="file-list"
+  >
+    <li>
+      <vaadin-upload-file
+        complete=""
+        tabindex="0"
+      >
+      </vaadin-upload-file>
+    </li>
+    <li>
+      <vaadin-upload-file tabindex="0">
+      </vaadin-upload-file>
+    </li>
+    <li>
+      <vaadin-upload-file
+        error=""
+        tabindex="0"
+      >
+      </vaadin-upload-file>
+    </li>
+    <dom-repeat
+      as="file"
+      style="display: none;"
+    >
+      <template is="dom-repeat">
+      </template>
+    </dom-repeat>
+  </ul>
+</slot>
+<slot>
+</slot>
+<input
+  accept=""
+  hidden=""
+  id="fileInput"
+  multiple=""
+  type="file"
+>
+`;
+/* end snapshot vaadin-upload shadow default */
+
diff --git a/packages/upload/test/dom/vaadin-upload-file.test.js b/packages/upload/test/dom/vaadin-upload-file.test.js
@@ -0,0 +1,22 @@
+import { expect } from '@esm-bundle/chai';
+import { fixtureSync } from '@vaadin/testing-helpers';
+import '../../src/vaadin-upload-file.js';
+
+describe('vaadin-upload-file', () => {
+  let uploadFile;
+
+  beforeEach(() => {
+    uploadFile = fixtureSync('<vaadin-upload-file></vaadin-upload-file>');
+    uploadFile.file = {
+      name: 'Workflow.pdf',
+      progress: 60,
+      status: '19.7 MB: 60% (remaining time: 00:12:34)'
+    };
+  });
+
+  describe('shadow', () => {
+    it('default', async () => {
+      await expect(uploadFile).shadowDom.to.equalSnapshot();
+    });
+  });
+});
diff --git a/packages/upload/test/dom/vaadin-upload.test.js b/packages/upload/test/dom/vaadin-upload.test.js
@@ -0,0 +1,27 @@
+import { expect } from '@esm-bundle/chai';
+import { fixtureSync, nextFrame } from '@vaadin/testing-helpers';
+import '../../src/vaadin-upload.js';
+
+describe('vaadin-upload', () => {
+  let upload;
+
+  beforeEach(async () => {
+    upload = fixtureSync('<vaadin-upload></vaadin-upload>');
+    upload.files = [
+      { name: 'Annual Report.docx', complete: true },
+      {
+        name: 'Workflow.pdf',
+        progress: 60,
+        status: '19.7 MB: 60% (remaining time: 00:12:34)'
+      },
+      { name: 'Financials.xlsx', error: 'An error occurred' }
+    ];
+    await nextFrame();
+  });
+
+  describe('shadow', () => {
+    it('default', async () => {
+      await expect(upload).shadowDom.to.equalSnapshot();
+    });
+  });
+});
diff --git a/packages/upload/theme/lumo/vaadin-upload-styles.js b/packages/upload/theme/lumo/vaadin-upload-styles.js
@@ -62,6 +62,10 @@ registerStyles(
       line-height: 1;
       vertical-align: -0.25em;
     }
+
+    [part='file-list'] > *:not(:first-child) > * {
+      border-top: 1px solid var(--lumo-contrast-10pct);
+    }
   `,
   { moduleId: 'lumo-upload' }
 );
@@ -71,8 +75,13 @@ const uploadFile = css`
     padding: var(--lumo-space-s) 0;
   }
 
-  :host(:not(:first-child)) {
-    border-top: 1px solid var(--lumo-contrast-10pct);
+  :host(:focus) {
+    outline: none;
+  }
+
+  :host(:focus) [part='row'] {
+    border-radius: var(--lumo-border-radius-s);
+    box-shadow: 0 0 0 2px var(--lumo-primary-color-50pct);
   }
 
   [part='row'] {
@@ -129,7 +138,6 @@ const uploadFile = css`
     cursor: var(--lumo-clickable-cursor);
   }
 
-  [part='row']:focus,
   [part$='button']:focus {
     outline: none;
     border-radius: var(--lumo-border-radius-s);