ENH Better use of ARIA attributes

silverstripe · Mar 28, 2024 · 0efdfdd · 0efdfdd
1 parent bc1f7c9
commit 0efdfdd
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 18 deletions.
diff --git a/client/dist/js/bundle.js b/client/dist/js/bundle.js
diff --git a/client/src/components/LinkField/tests/LinkField-test.js b/client/src/components/LinkField/tests/LinkField-test.js
@@ -99,10 +99,27 @@ test('LinkField will render disabled state if disabled is true', async () => {
   }) });
   await screen.findByText('Page title');
   const linkPicker = container.querySelectorAll('#link-picker__link-123');
-  expect(linkPicker[0]).toHaveAttribute('aria-disabled');
+  expect(linkPicker[0].getAttribute('aria-disabled')).toBe('true');
   expect(linkPicker[0]).toHaveClass('link-picker__link--disabled');
 });
 
+test('LinkField will have aria-disabled true if readonly is true', async () => {
+  const { container } = render(<LinkField {...makeProps({
+    ownerID: 1,
+    readonly: true
+  })}
+  />);
+  await doResolve({ json: () => ({
+    123: {
+      title: 'Page title',
+      typeKey: 'mylink',
+    }
+  }) });
+  await screen.findByText('Page title');
+  const linkPicker = container.querySelectorAll('#link-picker__link-123');
+  expect(linkPicker[0].getAttribute('aria-disabled')).toBe('true');
+});
+
 test('Empty disabled LinkField will display cannot edit message', async () => {
   const { container } = render(<LinkField {...makeProps({
     ownerID: 1,

diff --git a/client/src/components/LinkPicker/LinkPickerTitle.js b/client/src/components/LinkPicker/LinkPickerTitle.js
@@ -83,15 +83,6 @@ const LinkPickerTitle = ({
     event.preventDefault();
   };
 
-  const handleIconKeyDown = (event) => {
-    if (!['Enter', 'Space'].includes(event.code)) {
-      return;
-    }
-    const el = event.target;
-    const newVal = el.getAttribute('aria-pressed') === 'true' ? 'false' : 'true';
-    el.setAttribute('aria-pressed', newVal);
-  };
-
   const style = {
     transform: CSS.Transform.toString(transform),
     transition,
@@ -116,25 +107,24 @@ const LinkPickerTitle = ({
   if ([versionStates.draft, versionStates.modified].includes(versionState)) {
     onUnpublishedVersionedState();
   }
-  // Remove the default tabindex="0" attribute from the sortable element because we're going to manually
-  // add this to the drag handle instead
-  delete attributes.tabIndex;
+
   const idAttr = `link-picker__link-${id}`;
   return <div
     className={className}
     ref={setNodeRef}
     style={style}
-    {...attributes}
     {...listeners}
     id={idAttr}
+    aria-disabled={readonly || disabled}
   >
     { (isMulti && !readonly && !disabled) && <div className="link-picker__drag-handle"
         tabIndex="0"
         role="button"
-        aria-pressed="false"
+        aria-pressed={attributes['aria-pressed']}
+        aria-describedby={attributes['aria-describedby']}
+        aria-roledescription={attributes['aria-roledescription']}
         aria-controls={idAttr}
         aria-label="Sort Links"
-        onKeyDown={handleIconKeyDown}
     >
       <i
         className="font-icon-drag-handle"

diff --git a/client/src/components/LinkPicker/tests/LinkPickerTitle-test.js b/client/src/components/LinkPicker/tests/LinkPickerTitle-test.js
@@ -191,6 +191,5 @@ test('keydown on dnd handler', async () => {
   expect(container.querySelectorAll('.link-picker__drag-handle')).toHaveLength(1);
   container.querySelector('.link-picker__drag-handle').focus();
   fireEvent.keyDown(document.activeElement || document.body, { key: 'Enter', code: 'Enter', charCode: 13 });
-  expect(container.querySelector('.link-picker__drag-handle').getAttribute('aria-pressed')).toBe('true');
   expect(container.querySelector('.link-picker__drag-handle').getAttribute('aria-label')).toBe('Sort Links');
 });