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

ariaActivedescendant = false not valid aria #553

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

Jordan-Hall
Copy link
Contributor

@Jordan-Hall Jordan-Hall commented Apr 21, 2022

Fixes #434

I've used this as the base code for DLUHC. Needed to make small changes to get it to work with plotly dash so migrated from classes to hooks. This is one of the changes we've made to ensure lighthouse was 100.

PS: Happy to push the react hooks changes too if you are interested.

@romaricpascal romaricpascal added this to the Next milestone Jan 11, 2024
@romaricpascal
Copy link
Member

Hi @Jordan-Hall, thanks for contributing this fix. The changes look fine, but there are a couple of logistics bits that need to happen before we can merge them:

  • the branch needs to be rebased on main
  • dist needs to be re-built to grab the changes
  • The GitHub workflows need to run (hopefully pushing the first two changes should trigger that)

Would you be up for doing this or would you prefer I did it (appreciate it took us a bit for us to get to review this, so very fine to do it, thanks for your patience, btw).

On the topics of React hooks, thanks for offering, but we don't have any plans to move towards this. But if you have an open repository at DLUHC, this could be a handy place we could look into for making future decisions 😊

@Jordan-Hall
Copy link
Contributor Author

Jordan-Hall commented Jan 12, 2024

Thank you @romaricpascal I'll update the branch later. I left DLUHC now I was only contracting for them but still keep an eye on the repo I started for them. I completely rewrote alot of this to use hooks without losing any functionality.

https://github.com/communitiesuk/gov-uk-dash-react-components/blob/main/src/lib/fragments/AutoComplete.react.js I hope this useful for you. Here some of the changes and PRs we merged into it communitiesuk/gov-uk-dash-react-components#35

@romaricpascal
Copy link
Member

Brill! Thanks for the links and offering to update the branch 😊

@colinrotherham
Copy link
Contributor

I've rebased all open PRs with main (where we're able to) including this one

We'd caused a few conflicts by merging #600 first so that's all sorted now

@nicholasgriffintn
Copy link

Not related to this pr, but also sort of related, but thanks for getting this going again!

I'm gonna see if there's anything I can add from my fork when this is all done.

@colinrotherham
Copy link
Contributor

Needs a 2nd approval on this one @romaricpascal if you're happy that #553 (comment) is complete?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
4 participants