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

Feature request: the ability to turn off the error altogether #54

Closed
jamesplease opened this issue Jun 6, 2018 · 6 comments
Closed

Comments

@jamesplease
Copy link

jamesplease commented Jun 6, 2018

Hi there @davidtheclark ! To start, I'd like to thank you for sharing this amazing library. It's been really useful for me today.

I'm writing because I'm wondering if it would be possible to completely disable the error that this library throws when there are no focusable elements?

I can see why one may want an error when there are no focusable elements, but other times, it may be OK.

To provide some more details, my specific use case is a dropdown without any items. The docs suggest providing a fallback element, and I can see how that could work in some situations. However, I'm using this with React, and I don't want to make any assumptions about the existence of a ref.

This is for a production app at work, and I'd feel much more confident in my code if I had the guarantee that the error was disabled altogether.

What do you think, @davidtheclark ? Is there any way for the error to be disabled? I'd be happy to open a PR if you think that it's technically possible, and also worth adding.

Thanks for reading, and thank you again for creating such a great library!

--

Update: My current solution is to use fallbackFocus: document.body everywhere. It works inasmuch as it prevents explosions, but it isn't particularly expressive, and breaks the trap (tabbing still works). I think it could still be worthwhile to update the API of the library to support this in a less "hacky" feeling way ✌️

@davidtheclark
Copy link
Collaborator

@jamesplease Glad this library is useful for you!

How about adding a try-catch around the initialization? That seems to me as good as setting an option.

One question about your use case: If the dropdown is empty, where do you want focus to go?

@jamesplease
Copy link
Author

How about adding a try-catch around the initialization? That seems to me as good as setting an option.

I'm using the React bindings, so it's not clear to me where I would add that try-catch. Would it be best if I forked that library and added the try-catch myself?

One question about your use case: If the dropdown is empty, where do you want focus to go?

Good question! Because there are no elements that the user can activate, I am thinking that it follows that nothing be focused, and for tabbing to do nothing.

To close the dropdown, users can either hit Escape or Enter on their keyboard, or click outside of it. Then, after it is closed, I am manually returning the focus to the previously-focused item after the FocusTrap'd element is closed.

Maybe that's a little odd, though. Perhaps I'll propose a change to the UX where the dropdown is disabled when there are no items, and the explanation for the disabled state is provided in a tooltip on hover. For now, that explanation for the empty list is provided when the dropdown opens up.

@davidtheclark
Copy link
Collaborator

Because there are no elements that the user can activate, I am thinking that it follows that nothing be focused, and for tabbing to do nothing.

I'm not sure that nothing be focused is a good user experience to aim for. That leaves the keyboard user stranded in outer space, right? So I guess I'm still not convinced that it's a good idea to create a focus trap without a focusable element — still think that's probably an error that should be recognized as such.

@jamesplease
Copy link
Author

jamesplease commented Jun 10, 2018

I'm not sure that nothing be focused is a good user experience to aim for.

I agree that it's questionable 🙂

That leaves the keyboard user stranded in outer space, right?

In my particular situation, it doesn't. From my earlier comment:

"To close the dropdown, users can either hit Escape or Enter on their keyboard"

still think that's probably an error that should be recognized as such.

Perhaps. My bar of throwing errors is pretty high, and even higher for React components, where the developer can sometimes effectively lose all control over being able to try/catch it altogether, as in the case of the Focus Trap react wrapper.


It is interesting, because in my particular situation the dropdown represents dynamic processes, and the options available to the user may come and go depending on what is happening elsewhere (the options available are kept in sync with the server). So it is possible that you could open a dropdown with 3 options, but then, if you sit there long enough, it drops to 0.

Perhaps the UX to go with here does two things:

  1. Lets the user know that there are no actions to take
  2. Provides a button to close the dropdown, and that is what takes the focus

@davidtheclark
Copy link
Collaborator

Perhaps the UX to go with here

Yeah, I think that this error represents an accessibility problem to be addressed rather than avoided. Every time this request has come up so far that seems to have been the case.

@jamesplease
Copy link
Author

jamesplease commented Jun 17, 2018

Whats interesting to me is that native selects behave pretty similarly to the UX I first described in this issue.

To reproduce, create a native select without any options, and open it. Click Tab and observe that it is trapped, but it behaves as if nothing is selected (although it is likely the select itself that is focused). Note: I only tested this in Safari on macOS.

Anyway, it is pretty clear to me that unmounting a React app (or a section of a React app) due to focus is never a good idea, so if you’re not open to any PRs to make adjustments I’ll go ahead and create a fork that’s more safe to use in React apps.

Thanks for the good discussion @davidtheclark !

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

No branches or pull requests

2 participants