search

wellcomecollection / wellcomecollection.org

🪟 Wellcome Collection's website and services that support it
https://wellcomecollection.org
MIT License
37 stars 5 forks source link

Detect when CookieControl cookie has been set to change Focus Trap behaviour #10915

Closed rcantin-w closed 2 weeks ago

rcantin-w commented 1 month ago

Image

When the cookie banner shows on a page with a Modal (such as this work), the latter is stopping the former from being interacted with as it is locked in with FocusTrap (for a11y reasons).

We need to deactivate FocusTrap if the cookie banner has yet to be acknowledged and interacted with, then re-activate it to stick by good a11y practices.

Slack convo Slack convo 2

rcantin-w commented 1 month ago

The first PR #10920 fixed it perfectly locally, but on live environments still had issues on first load. https://github.com/wellcomecollection/wellcomecollection.org/pull/10921 was created in an effort to remedy that, and it did work to an extent but line 238 was a miss (I wanted to wrap it in else, not just have it fire every time) that made it so the Modal never had focus again, which is really bad for screen readers and keyboard navigation.

So I tried to revert but now the Check URL test is failing. I think it's not linked to the work since this is just a revert, and the error says

WARN[0000] The "NEXT_PUBLIC_CIVICUK_API_KEY" variable is not set. Defaulting to a blank string.

So it looks like it might have to do with the fetching of that key instead?

EDIT: Robert's confirmed it's unrelated and that error is expected to print at the moment.

rcantin-w commented 1 month ago

After trying to figure it out by running other e2e tests in other PRs, it looks like it was just a fluke? No idea why it repeatedly failed; Check URL was saying everything returned 503s even though the site was up 🤷‍♀
I pushed a new small commit (change to comments) and it reran the tests fine, so I'm merging the reverted code now.

The original issue will need further work to fix properly so leaving the ticket open.

rcantin-w commented 2 weeks ago

Looks to be fully fixed in prod now.