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

Add accesibility links for supported councils #695

Merged
merged 2 commits into from
Apr 22, 2024
Merged

Conversation

symroe
Copy link
Member

@symroe symroe commented Apr 18, 2024

Very rough and ready, I don't really know how React works.

Copy link

This PR has been deployed.

Copy link

@GeoWill GeoWill left a comment

Choose a reason for hiding this comment

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

Well I found your intentional error ;)

I confess the rest is pretty confusing! Not sure how usefully I can review just by reading.
Would it be worth me setting it up locally and try it out?

I'm particularly confused by why the widget wasn't already handling UPRNs, or is it more a case of shuffling the UPRN to where you need it in order to link back to WDIV when there's a split postcode?

src/PollingStation.js Outdated Show resolved Hide resolved
@VirginiaDooley
Copy link
Contributor

I had to upgrade several packages and refactor a few things to get this to run. Now that I have (happy to push those changes to another PR if you like), I don't think this change has been added to a sample postcode. If the expectation is to test on live data, I will need an API key.

This is a quick way to test the accessibility links without having to
mock the councils who have the link enabled
@symroe
Copy link
Member Author

symroe commented Apr 21, 2024

  • Duplicate council ID error fixed
  • S3 deployment fixed: this was due to a failing test that I'd not spotted
  • Example council ID changed, so AA12AA should now show this link.

I've not added the Welsh translation because I want feedback on what it should say. @pmk01?

@symroe symroe merged commit 43ead99 into master Apr 22, 2024
3 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