Skip to content

Limestone rewrite - #46

Open
Nealium104 wants to merge 100 commits into
mainfrom
limestone-rewrite
Open

Limestone rewrite#46
Nealium104 wants to merge 100 commits into
mainfrom
limestone-rewrite

Conversation

@Nealium104

@Nealium104 Nealium104 commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Related to this exploreuk-web-app PR, this pull request is a full UI rewrite of the findingaid. This rewrite is primarily for addressing accessibility concerns.

This PR fixes #7, fixes #8, fixes #9, fixes #10, fixes #22, fixes #24, fixes #33, fixes #34, fixes #35, fixes #26, fixes #57, fixes #58.

@Nealium104
Nealium104 force-pushed the limestone-rewrite branch from e75f573 to e3c7ccb Compare June 8, 2026 19:00
@Nealium104
Nealium104 marked this pull request as ready for review June 9, 2026 20:19
@Nealium104
Nealium104 requested a review from a team as a code owner June 9, 2026 20:19
@nrsa222
nrsa222 marked this pull request as draft June 25, 2026 17:28
@nrsa222

nrsa222 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

To do:

@nrsa222

nrsa222 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #57

@nrsa222
nrsa222 marked this pull request as ready for review August 18, 2026 20:44
@nrsa222

nrsa222 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Resolves #58 and resolves #57.

nrsa222
nrsa222 previously approved these changes Aug 18, 2026

@nrsa222 nrsa222 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Nealium104 - Great job, this is a major frontend update that makes the application look and feel like a part of the larger UK web presence. It fixes many accessibility issues and has improved the code base in a significant way. I left one accessibility suggestion but will approve this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For better accessibility, the button text should allow users to determine it's purpose from button text alone. I've found a good rule of thumb is to have action verb + noun, describing what the button does. In this specific case, I would just match the aria-label text.

@nrsa222 nrsa222 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Previous review was dismissed inadvertently when I pulled main into this branch to resolve a minor merge conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment