search

dxc-technology / halstack-react

Library of components for building SPAs with React and Halstack Design System
https://developer.dxc.com/halstack/
Apache License 2.0
15 stars 14 forks source link

Dialog review #1504

Closed GomezIvann closed 1 year ago

GomezIvann commented 1 year ago

Checklist

Description Summary:

  1. Extra padding-top removed (#1446). Now, when displayed, the decision of how to avoid the clear action button relies on the user.
  2. The content is now free from development decisions (#1470). The content is no longer subject to its container display value (removed flex). The decision of how to lay out the components inside the Dialog relies on the user.
  3. Responsive values updated. The width of the dialog when the screen is smaller changes from 800px to 696px.
  4. Fixed problems related to the Escape key. When the dialog didn't have any component to bubble the onkeydown event, the key didn't close the modal box. For example, a Dialog with only text.
  5. New implementation: Focus Lock. A new utility component was added to the DS that keeps the focus on its children and autofocuses the first available element.
  6. Removed several tokens related to the typography of the custom content: fontFamily, fontSize and fontWeight.
  7. Updated specifications with the new values in the tables. Also, removed the previously mentioned tokens from every place on the documentation.

Closes #1446 #1470 #1485 #1496

GomezIvann commented 1 year ago

These are some of the strange behaviours I have found out for now:

If there is an element with a tabIndex > 0 the focus can leave the dialogue. When using the shift tab from the first element it seems to need an extra step to focus on the last element.

The second case is already fixed. In regards to the first one, I couldn't reproduce it since in my code is working fine, could you please paste yours in a comment?

image

Jialecl commented 1 year ago

I just simply added a DxcButton with tabIndex={1} to the dialog. Related to this issue using the shift tab I was nerver able to focus on the button with positive tabIndex.

GomezIvann commented 1 year ago

I just simply added a DxcButton with tabIndex={1} to the dialog. Related to this issue using the shift tab I was nerver able to focus on the button with positive tabIndex.

I saw the problem. When we change the tabIndex of the inner content to a different value than the Dialog's one, it seems to lose the lock (understandable). I'm currently checking this since I'm not sure how it should work in this case.

Jialecl commented 1 year ago

I just simply added a DxcButton with tabIndex={1} to the dialog. Related to this issue using the shift tab I was nerver able to focus on the button with positive tabIndex.

I saw the problem. When we change the tabIndex of the inner content to a different value than the Dialog's one, it seems to lose the lock (understandable). I'm currently checking this since I'm not sure how it should work in this case.

One more thing I noticed is that if the first element has tabIndex > 0 it is still autofocused even if there are elements with higher priority.

GomezIvann commented 1 year ago

I just simply added a DxcButton with tabIndex={1} to the dialog. Related to this issue using the shift tab I was nerver able to focus on the button with positive tabIndex.

I saw the problem. When we change the tabIndex of the inner content to a different value than the Dialog's one, it seems to lose the lock (understandable). I'm currently checking this since I'm not sure how it should work in this case.

One more thing I noticed is that if the first element has tabIndex > 0 it is still autofocused even if there are elements with higher priority.

That's because that's not how tabIndex works. Here, the main goal is to autofocus the first available focusable item of the Dialog. By changing the value of its tabIndex you are only establishing a different order in the normal sequence of tabbable elements. tabIndex is not used for autofocusing, for that kind of purpose is the autofocus property (which is what we are trying to reproduce).

You can take a look at other implementations to check this:

  1. https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/examples/dialog/
  2. https://www.radix-ui.com/docs/primitives/components/dialog