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

[FINNA-452] Fix aria expanded on search facets #2657

Conversation

siiriylonen
Copy link

No description provided.

@siiriylonen
Copy link
Author

(En tiedä olisiko esim. 450, 451 ja 452 pr voinut yhdistää, mutta nyt meni näin.)

@siiriylonen siiriylonen marked this pull request as ready for review July 26, 2023 13:02
Copy link

@EreMaijala EreMaijala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bootstrap (accessible?) asettaa joka tapauksessa aria-expanded:n, kun avaa tai sulkee, joten tämä korjaus ei auta. Jos vika on Bootstrapissa, niin ei voi mitään, mutta ehkä auttaisi, jos div:llä olisi role="listbox"?

@pasitiis
Copy link

pasitiis commented Aug 2, 2023

Vaikuttaa jotenkin bootstrap-accessibility ongelmalta. Heidän esimerkeissä aria-expandedia ei aseta html:ää, tällöin menee myös axe:n validatorista läpi. https://paypal.github.io/bootstrap-accessibility-plugin/demo.html#collapse-examples .

Bootstrap ilman bootstrap-accesibilityä menee validorista läpi vaikka aria-expanded on asetettu.. https://getbootstrap.com/docs/3.3/javascript/#collapse

Copy link

@EreMaijala EreMaijala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pasin kanssa keskustelun jälkeen todettiin, ettei tätä nyt saada kokonaan kuntoon, kun vika on bootstrap-accessibility -moduulissa, mutta tämä korjaus auttaa hiukan lähtötilanteeseen, joten mennään tällä nyt toistaiseksi.

@EreMaijala EreMaijala removed the request for review from pasitiis August 2, 2023 09:38
@EreMaijala EreMaijala merged commit 4def71a into NatLibFi:dev Aug 2, 2023
6 checks passed
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

Successfully merging this pull request may close these issues.

3 participants