Skip to content
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

Don't add tab index to empty drop zones #245

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Don't add tab index to empty drop zones #245

wants to merge 3 commits into from

Conversation

danny-andrews
Copy link
Contributor

@danny-andrews danny-andrews commented Feb 7, 2021

Currently, tabindex = 0 is being added to the drop zone even when no element is being dragged:

node.tabIndex =
isDragging &&
(node === focusedDz ||
focusedItem.contains(node) ||
config.dropFromOthersDisabled ||
(focusedDz && config.type !== dzToConfig.get(focusedDz).type))
? -1
: 0;

Then, in the focus handler, we do nothing if dragging is not in progress:

function handleZoneFocus(e) {
printDebug(() => "zone focus");
if (!isDragging) return;

I don't think it makes sense to make an element focusable if we do nothing when it takes focus. Plus, it leads to weird styling on empty lists. (See screenshot below.)

image

@danny-andrews
Copy link
Contributor Author

Okay, never-mind, this breaks accessibility. Do you have any thoughts on how to handle the empty-list problem, though?

@danny-andrews
Copy link
Contributor Author

I updated the PR to set tabindex to -1 when there are no items in the drag zone.

@danny-andrews danny-andrews changed the title Don't add tab index to drop zone when isDragging is false Don't add tab index to empty drop zones Feb 7, 2021
@isaacHagoel
Copy link
Owner

Hi @danny-andrews ,
Thanks for this and sorry for the delay. i was swamped this week.
the question is: what if there are some instructions on the list that are meaningful even when it's empty.
for example:
maybe you have a component with 4 lists, the first one is an items bank that has all of the items initially and the others are categories that the user needs to sort the items into (imagine it is a question in a quiz).
in this case it is important for the user to know about the existence of the empty lists and their titles which can be done by tabbing around.
Thoughts?

I've been thinking for awhile that we probably need to allow to override the tabindex that is set by the library but it's not fully formed in my mind.

@danny-andrews
Copy link
Contributor Author

Not a problem.

Yeah, that's a fair point. I wonder if we could do away with adding a tabindex to the dragzone altogether and add aria-describedby to each of the list items, along with using live regions to update them when the state changes: https://medium.com/salesforce-ux/4-major-patterns-for-accessible-drag-and-drop-1d43f64ebf09#0303.

@isaacHagoel
Copy link
Owner

isaacHagoel commented Feb 12, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants