search

ASU / asu-unity-stack

ASU Unity Design System
https://unity.web.asu.edu/
Other
11 stars 8 forks source link

UDS-1775: fix(unity-bootstrap-theme): switch anchor to button on accordions header and added styles #1335

Closed juanmitriatti closed 1 month ago

juanmitriatti commented 1 month ago

fix(unity-bootstrap-theme): switch anchor to button on accordions header and added styles

UDS-1775

Description

Solution

Switch over to use buttons instead of anchor tags ad that solves it for us without the need for custom javascript.

Links

FOR APPROVERS

juanmitriatti commented 1 month ago

2 issues:

  1. In testing this, I'm not seeing the focus state for the accordion header/button, and that's a regression. See the "states" tab for how that should be styeld: https://zeroheight.com/9f0b32a56/p/39a26f-accordion
  2. There is a React version of the accordion in components-core and it will need to get updated as well. The commit to add that update should use the components-core scope so it triggers a release for that package when this is merged. If you have questions about this version of the accordion, please reach out to Dave or Scott. https://asu.github.io/asu-unity-stack/@asu/components-core/index.html?path=/story/uds-accordion--default

Hi @mlsamuelson Regarding this first concern, please check these images :

Image 1 : This is what I locally see using the new approach "w/ button: focus enabled in h4 or button" image

Image 2 : This is what I see using the previous approach "w/ anchor, focus enabled in h4 tag" image

Would you please explain me the error that I'm introducing here? Thanks!

mlsamuelson commented 1 month ago

Thanks for the question, @juanmitriatti . As we discovered when we Zoomed about this issue, it turns out that the missing focus state is a problem in the current unity-bootstrap-theme and components-core version of the accordions. It makes sense to fix it here.

Please also check the Open and Close toggle on Accordions in multiple browsers. In my testing, the accordions only open in Chrome, but in Firefox they both open and close. The Firefox behavior is correct.

mlsamuelson commented 1 month ago

Once we have a commit for the components-core scope, this should be good.

juanmitriatti commented 1 month ago

Hi @mlsamuelson This is done. Please review the last commit. Thanks!