-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SITES-2864: Accessibility - Drag and Drop feature is not keyboard accessible. #242
base: master
Are you sure you want to change the base?
Conversation
We will have to do some additional work to support Browse/Virtual Cursor mode with NVDA or JAWS. NVDA or JAWS will not automatically switch into Forms mode when the user presses Space or Enter on a button to start dragging, which means that while the user may expect the Up/Down arrow keys to move the item after starting the drag operation, NVDA or JAWS will continue to interpret the arrow keys as screen reader navigation. A solution would be to wrap the drag handle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes seem to break the drag and drop behavior when using the mouse. In the Table examples, one can drag and drop to reorder a single row, but if you mouse down on another drag handle, the style changes to start reordering, but one cannot drag to reorder, and the "grabbed" cursor styling doesn't change.
|
||
events.on(`touchmove.DragAction${this._id}`, this._drag); | ||
events.on(`mousemove.DragAction${this._id}`, this._drag); | ||
events.on(`touchend.DragAction${this._id}`, this._dragEnd); | ||
events.on(`mouseup.DragAction${this._id}`, this._dragEnd); | ||
events.on(`focusout.DragAction${this._id}`, this._dragOnKeyEnd); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this focusout
event handler to only after the user has started a keyboard drag and drop operation.
events.on(`focusout.DragAction${this._id}`, this._dragOnKeyEnd); |
this._dragEvents.dispatch('coral-dragaction:dragonkeyspace', { | ||
detail: { | ||
dragElement: this._dragElement | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the focusout
event listener when the keyboard drag and drop operation starts.
this._dragEvents.dispatch('coral-dragaction:dragonkeyspace', { | |
detail: { | |
dragElement: this._dragElement | |
} | |
}); | |
this._dragEvents.dispatch('coral-dragaction:dragonkeyspace', { | |
detail: { | |
dragElement: this._dragElement | |
} | |
}); | |
events.on(`focusout.DragAction${this._id}`, this._dragOnKeyEnd); |
this._dragEvents.dispatch('coral-dragaction:dragendonkey', { | ||
detail: { | ||
dragElement: this._dragElement | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the focusout
event handler when the keyboard drag and drop operation ends.
this._dragEvents.dispatch('coral-dragaction:dragendonkey', { | |
detail: { | |
dragElement: this._dragElement | |
} | |
}); | |
this._dragEvents.dispatch('coral-dragaction:dragendonkey', { | |
detail: { | |
dragElement: this._dragElement | |
} | |
}); | |
events.off(`focusout.DragAction${this._id}`, this._dragOnKeyEnd); |
'coral-dragaction:dragend tbody[is="coral-table-body"] tr[is="coral-table-row"]': '_onRowDragEnd', | ||
'coral-dragaction:dragend tbody[is="coral-table-body"] tr[is="coral-table-row"]': '_onRowDragEnd', | ||
// a11y dnd | ||
'key:space tbody[is="coral-table-body"] [coral-table-roworder]:not([disabled])': '_onKeyboardDrag', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a click
event handler to the drag handle, which will allow a screen reader user to activate the control to initiate the drag operation starting from browse/virtual cursor mode.
'key:space tbody[is="coral-table-body"] [coral-table-roworder]:not([disabled])': '_onKeyboardDrag', | |
'key:space tbody[is="coral-table-body"] [coral-table-roworder]:not([disabled])': '_onKeyboardDrag', | |
'click tbody[is="coral-table-body"] tr[is="coral-table-row"] [coral-table-roworder]:not([disabled])': '_onDragHandleClick', |
const table = this; | ||
const row = event.target.closest('tr[is="coral-table-row"]'); | ||
|
||
if (row && table.orderable) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- preventDefault and stopPropagation so that the Space key does not start scrolling the page.
- Wrap the drag handle button in a span with role="application", to force Windows screen readers into forms mode while dragging.
- Set the selected state of the drag handle and add
aria-pressed="true"
to better communicate the dragging state.
if (row && table.orderable) { | |
if (row && table.orderable) { | |
event.preventDefault(); | |
event.stopImmediatePropagation(); | |
// Wrap the drag handle button in a span with role="application", | |
// to force Windows screen readers into forms mode while dragging. | |
const handle = row.querySelector('[coral-table-roworder]'); | |
if (handle) { | |
if (handle.getAttribute('is') === 'coral-button') { | |
handle.selected = true; | |
handle.setAttribute('aria-pressed', 'true'); | |
} | |
if (event.target === handle && !handle.closest('span[role="application"]')) { | |
const span = document.createElement('span'); | |
span.setAttribute('role', 'application'); | |
handle.parentNode.insertBefore(span, handle); | |
span.appendChild(handle); | |
window.requestAnimationFrame(function() { | |
handle.focus(); | |
}); | |
} | |
} | |
const dragElement = event.detail.dragElement; | ||
const items = getRows([body]); | ||
const index = getIndexOf(dragElement); | ||
const dragData = dragElement.dragAction._dragData; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preventDefault to prevent scrolling of page when reordering.
const dragData = dragElement.dragAction._dragData; | |
const dragData = dragElement.dragAction._dragData; | |
const handle = dragElement.dragAction.handle; | |
event.preventDefault(); |
// Restore specific styling | ||
dragElement.setAttribute('style', dragData.style.row || ''); | ||
getCells(dragElement).forEach((cell, i) => { | ||
cell.setAttribute('style', dragData.style.cells[i] || ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix for a console error:
cell.setAttribute('style', dragData.style.cells[i] || ''); | |
if (dragData.style.cells) { | |
cell.setAttribute('style', dragData.style.cells[i] || ''); | |
} |
dragElement.setAttribute('style', dragData.style.row || ''); | ||
getCells(dragElement).forEach((cell, i) => { | ||
cell.setAttribute('style', dragData.style.cells[i] || ''); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restore focus to the handle and scroll the row into view after reordering.
Note: We should probably announce the new position of the row within the table using a live region announcement at this point.
}); | |
}); | |
handle.focus(); | |
dragElement.scrollIntoView({block: 'nearest'}); |
_onRowDragOverOnKeyEnter(event) { | ||
const table = this; | ||
const dragElement = event.detail.dragElement; | ||
dragElement.dragAction.destroy(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dragElement.dragAction.destroy(); | |
const dragAction = dragElement.dragAction; | |
const handle = dragAction.handle; | |
const span = handle && handle.closest('span[role="application"]'); | |
// If keyboard dragging, do nothing, and wait for event to be triggered from DragAction. | |
if (dragAction._isKeyboardDrag) { | |
dragAction._isKeyboardDrag = undefined; | |
return; | |
} | |
if (handle && handle.getAttribute('is') === 'coral-button') { | |
handle.selected = false; | |
handle.removeAttribute('aria-pressed'); | |
} | |
dragAction.destroy(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to dispatch coral-table:reorder
events:
|
||
// Refocus the dragged element manually | ||
window.requestAnimationFrame(() => { | ||
dragElement.classList.remove('is-focused'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean up the span
that forced forms mode.
dragElement.classList.remove('is-focused'); | |
if (span) { | |
span.parentNode.insertBefore(handle, span); | |
span.remove(); | |
} | |
dragElement.classList.remove('is-focused'); |
- force forms mode while dragging - ignore "virtual" click to start dragging on mousedown or touchstart
- force forms mode while dragging - ignore "virtual" click to start dragging on mousedown or touchstart
- force forms mode while dragging - ignore "virtual" click to start dragging on mousedown or touchstart
- force forms mode while dragging - ignore "virtual" click to start dragging on mousedown or touchstart
- force forms mode while dragging - announce using a live region after reorder - fire coral-table:roworder on reorder.
Changes have been moved to #346 along with the suggested updates. This PR can now be closed |
Description
Jira Ticket: https://jira.corp.adobe.com/browse/SITES-2864
Fixing this issue with the following approach:
space
key - capture the element and opens keyboard drag&drop functionalityarrowUp
andarrowDown
keys - swaps the element (more exactly the draggable row) with previous or next elemententer
key - stops the drag&drop functionality (also this is possible when losing focus from the element)Related Issue
No overview regarding this.
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: