Trendyol / baklava

Baklava is a design system provided by Trendyol to create a consistent UI/UX for app users.
https://baklava.design/
MIT License
1.26k stars 112 forks source link

[BUG]: Dialog does not trap focus #797

Open bdougherty opened 7 months ago

bdougherty commented 7 months ago

Issue description

I was using the documentation page: https://baklava.design/?path=/docs/components-dialog--documentation

  1. Open the dialog
  2. Hit tab repeatedly
  3. Focus will move outside the dialog to the page in the background

This is not good behavior, and something that is handled properly by <dialog>, which I notice this component does not use.

Media & Screenshots

No response

Baklava Version

No response

Operating system

No response

Priority this issue should have

Medium (should be fixed soon)

Please review the checkboxes that are applicable.

ozkersemih commented 7 months ago

I tried to solve this issue with some methods but there are problems: 1- We can solve focus trapping by listening keyboard events when dialog is opened. If we set first and last focusable element in dialog, we can keep tab and shift+tab focusing behaviour only in bl-dialog component. But the problem is we can't get focusable elements properly because of shadow dom. Yes of course we can just get close and done buttons in dialog but there can be another focusable elements inside dialogs content area.

2- Another solution is using inert attribute. When bl-dialog is opened, we can add inert attribute for everything except opened bl-dialog. Inert attribute make an element as non-focusable/disable. But the problem is shadow dom and parent-child hierarchy in this solution. For example if bl-dialog under a parent div and it has siblings, we shouldn't set parents of bl-dialog as inert, we should only set its siblings.

fatihhayri commented 7 months ago

Native dialog element has no have a this problem.

polyfilled="false" attribute solve this problem.