search

GavinJoyce / ember-headlessui

https://gavinjoyce.github.io/ember-headlessui/
Other
92 stars 34 forks source link

➖ Remove ember focus trap addon #98

Closed far-fetched closed 3 years ago

far-fetched commented 3 years ago

We still have some skipped / todo tests in dialog, listbox, menu(?) which are related to focused element after close component or click outside. dialog test listbox

To sort them out, I added some useful functionality to focus-trap package: link Last step would be to update ember-focus-trap with new version of focus-trap. When I tried to do it locally and fix remaining tests, I figured out that ember-focus-trap adds some extra functionality to it. Currently, we don't use any of them + one of the functionality causes a bug, which I described here. TL TR: ember-focus-trap replicates behavior which is already implemented in focus-trap

With this issue I think we are not able to fix those remaining tests. This PR removes ember-focus-trap in favor of own bare minimum implementation. Tests are still passing, so I assume that I didn't change any behavior.

GavinJoyce commented 3 years ago

Thanks, is the plan to re-enable the skipped tests?

far-fetched commented 3 years ago

Yep I can do this, just after we merge this PR which updates focus-trap dep with new functionality.

GavinJoyce commented 3 years ago

👍 sounds great, thanks