Skip to content

Commit 52985fd

Browse files
jonathanwelch1-nhsjonathanwelch1-nhs
authored
Fix: [AEA-5081] - header plus others (#1709)
## Summary https://nhsd-jira.digital.nhs.uk/browse/AEA-5081 https://nhsd-jira.digital.nhs.uk/browse/AEA-5364 - Routine Change ### Details fixed skip link made skip link always the first clickable hero banner colouring fixed fixed dropdown styling on hover fixed broken aria label made the spinner announce loading and improved its accessibility
1 parent 325fa0f commit 52985fd

File tree

18 files changed

+212
-112
lines changed

18 files changed

+212
-112
lines changed

‎packages/common/commonTypes/src/index.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,5 @@ export type {
3636
TrackerUserInfoResult
3737
} from "./trackerUserInfo"
3838

39-
export {Headers, ApigeeConfig} from "./headers"
39+
export {Headers} from "./headers"
40+
export type {ApigeeConfig} from "./headers"

‎packages/cpt-ui/__tests__/EpsPrescriptionList.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ describe("PrescriptionListPage", () => {
379379
}
380380
)
381381

382-
expect(screen.getByText("Loading...")).toBeVisible()
382+
expect(screen.getByTestId("spinner")).toBeInTheDocument()
383383
})
384384

385385
it("renders the component with the correct title and heading", async () => {

‎packages/cpt-ui/__tests__/EpsSearchForAPrescription.test.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,9 @@ describe("SearchForAPrescription", () => {
118118

119119
it("renders hero container with proper styling", () => {
120120
renderWithProviders(<SearchForAPrescription />)
121-
const heroContainer = screen.getByTestId("search-hero-container")
121+
const heroContainer = screen.getByTestId("hero-banner")
122122
expect(heroContainer).toBeInTheDocument()
123-
expect(heroContainer).toHaveClass("hero-container")
123+
expect(heroContainer).toHaveClass("nhsuk-hero-wrapper")
124124
})
125125

126126
it("sets active tab based on pathname - prescription ID", () => {

‎packages/cpt-ui/__tests__/LogoutPage.test.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ describe("LogoutPage", () => {
118118
)
119119

120120
expect(screen.getByText(/Logging out/i)).toBeInTheDocument()
121-
expect(screen.getByRole("progressbar")).toBeInTheDocument()
121+
expect(screen.getByRole("status")).toBeInTheDocument()
122122
})
123123

124124
it("does not call signOut if user is signed in, but we haven't advanced timers yet", () => {
@@ -129,6 +129,6 @@ describe("LogoutPage", () => {
129129
)
130130

131131
expect(screen.getByText(/Logging out/i)).toBeInTheDocument()
132-
expect(screen.getByRole("progressbar")).toBeInTheDocument()
132+
expect(screen.getByRole("status")).toBeInTheDocument()
133133
})
134134
})

‎packages/cpt-ui/__tests__/SearchPrescriptionPageEnhanced.test.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,9 @@ describe("SearchForAPrescription", () => {
155155

156156
it("renders hero container with proper styling", () => {
157157
renderWithProviders(<SearchForAPrescription />)
158-
const heroContainer = screen.getByTestId("search-hero-container")
158+
const heroContainer = screen.getByTestId("hero-banner")
159159
expect(heroContainer).toBeInTheDocument()
160-
expect(heroContainer).toHaveClass("hero-container")
160+
expect(heroContainer).toHaveClass("nhsuk-hero-wrapper")
161161
})
162162

163163
it("renders search tabs container", () => {

‎packages/cpt-ui/__tests__/SearchPrescriptionPagePaths.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ describe("SearchPrescriptionPage - Path and UseEffect Coverage", () => {
293293
it("renders content wrapper and containers with correct test ids", () => {
294294
const {container} = renderWithProviders(<SearchPrescriptionPage />)
295295

296-
expect(container.querySelector('[data-testid="search-hero-container"]')).toBeInTheDocument()
296+
expect(container.querySelector('[data-testid="hero-banner"]')).toBeInTheDocument()
297297
expect(container.querySelector('[data-testid="search-tabs-container"]')).toBeInTheDocument()
298298
expect(container.querySelector('[data-testid="hero-banner"]')).toBeInTheDocument()
299299
})

‎packages/cpt-ui/src/App.tsx‎

Lines changed: 68 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {NavigationProvider} from "@/context/NavigationProvider"
66
import {PatientDetailsProvider} from "./context/PatientDetailsProvider"
77
import {PrescriptionInformationProvider} from "./context/PrescriptionInformationProvider"
88
import Layout from "@/Layout"
9+
import {useEffect} from "react"
910

1011
import LoginPage from "@/pages/LoginPage"
1112
import LogoutPage from "@/pages/LogoutPage"
@@ -33,6 +34,34 @@ import {HEADER_STRINGS} from "@/constants/ui-strings/HeaderStrings"
3334
function AppContent() {
3435
const location = useLocation()
3536

37+
// this useEffect ensures that focus starts with skip link when using tab navigation
38+
useEffect(() => {
39+
let hasTabbed = false
40+
41+
const activeElement = document.activeElement as HTMLElement
42+
if (activeElement && activeElement !== document.body) {
43+
activeElement.blur()
44+
}
45+
46+
const handleKeyDown = (e: KeyboardEvent) => {
47+
if (e.key === "Tab" && !hasTabbed && !e.shiftKey) {
48+
hasTabbed = true
49+
e.preventDefault()
50+
const skipLink = document.querySelector(".nhsuk-skip-link") as HTMLElement
51+
if (skipLink) {
52+
skipLink.focus()
53+
}
54+
document.removeEventListener("keydown", handleKeyDown)
55+
}
56+
}
57+
58+
document.addEventListener("keydown", handleKeyDown)
59+
60+
return () => {
61+
document.removeEventListener("keydown", handleKeyDown)
62+
}
63+
}, [location.pathname])
64+
3665
// Check if we're on a prescription list or prescription details page
3766
const isPrescriptionPage =
3867
location.pathname === FRONTEND_PATHS.PRESCRIPTION_LIST_CURRENT ||
@@ -43,51 +72,53 @@ function AppContent() {
4372
const skipTarget = isPrescriptionPage ? "#patient-details-banner" : "#main-content"
4473

4574
return (
46-
<PatientDetailsProvider>
47-
<EPSCookieBanner />
75+
<>
4876
<a
4977
href={skipTarget}
5078
className="nhsuk-skip-link"
5179
data-testid="eps_header_skipLink"
5280
>
5381
{HEADER_STRINGS.SKIP_TO_MAIN_CONTENT}
5482
</a>
55-
<PrescriptionInformationProvider>
56-
<SearchProvider>
57-
<NavigationProvider>
58-
<Routes>
59-
<Route path="/" element={<Layout />}>
60-
{/* Public cookie routes */}
61-
<Route path="cookies" element={<CookiePolicyPage />} />
62-
<Route path="cookies-selected" element={<CookieSettingsPage />} />
83+
<PatientDetailsProvider>
84+
<EPSCookieBanner />
85+
<PrescriptionInformationProvider>
86+
<SearchProvider>
87+
<NavigationProvider>
88+
<Routes>
89+
<Route path="/" element={<Layout />}>
90+
{/* Public cookie routes */}
91+
<Route path="cookies" element={<CookiePolicyPage />} />
92+
<Route path="cookies-selected" element={<CookieSettingsPage />} />
6393

64-
{/* Your existing routes */}
65-
<Route path="*" element={<NotFoundPage />} />
66-
<Route path={FRONTEND_PATHS.SESSION_SELECTION} element={<SessionSelectionPage />} />
67-
<Route path={FRONTEND_PATHS.LOGIN} element={<LoginPage />} />
68-
<Route path={FRONTEND_PATHS.LOGOUT} element={<LogoutPage />} />
69-
<Route path={FRONTEND_PATHS.SESSION_LOGGED_OUT} element={<SessionLoggedOutPage />} />
70-
<Route path={FRONTEND_PATHS.SELECT_YOUR_ROLE} element={<SelectYourRolePage />} />
71-
<Route path={FRONTEND_PATHS.YOUR_SELECTED_ROLE} element={<YourSelectedRolePage />} />
72-
<Route path={FRONTEND_PATHS.CHANGE_YOUR_ROLE} element={<ChangeRolePage />} />
73-
<Route path={FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID} element={<SearchPrescriptionPage />} />
74-
<Route path={FRONTEND_PATHS.SEARCH_BY_NHS_NUMBER} element={<SearchPrescriptionPage />} />
75-
<Route path={FRONTEND_PATHS.SEARCH_BY_BASIC_DETAILS} element={<SearchPrescriptionPage />} />
76-
<Route path={FRONTEND_PATHS.PRESCRIPTION_LIST_CURRENT} element={<PrescriptionListPage />} />
77-
<Route path={FRONTEND_PATHS.PRESCRIPTION_LIST_FUTURE} element={<PrescriptionListPage />} />
78-
<Route path={FRONTEND_PATHS.PRESCRIPTION_LIST_PAST} element={<PrescriptionListPage />} />
79-
<Route path={FRONTEND_PATHS.PRESCRIPTION_DETAILS_PAGE} element={<PrescriptionDetailsPage />} />
80-
<Route path={FRONTEND_PATHS.PATIENT_SEARCH_RESULTS} element={<SearchResultsPage />} />
81-
<Route path={FRONTEND_PATHS.TOO_MANY_SEARCH_RESULTS} element={<TooManySearchResultsPage />} />
82-
<Route path={FRONTEND_PATHS.NO_PATIENT_FOUND} element={<NoPatientsFoundPage />} />
83-
<Route path={FRONTEND_PATHS.NO_PRESCRIPTIONS_FOUND} element={<NoPrescriptionsFoundPage />} />
84-
<Route path={FRONTEND_PATHS.PRIVACY_NOTICE} element={<PrivacyNoticePage />} />
85-
</Route>
86-
</Routes>
87-
</NavigationProvider>
88-
</SearchProvider>
89-
</PrescriptionInformationProvider>
90-
</PatientDetailsProvider>
94+
{/* Your existing routes */}
95+
<Route path="*" element={<NotFoundPage />} />
96+
<Route path={FRONTEND_PATHS.SESSION_SELECTION} element={<SessionSelectionPage />} />
97+
<Route path={FRONTEND_PATHS.LOGIN} element={<LoginPage />} />
98+
<Route path={FRONTEND_PATHS.LOGOUT} element={<LogoutPage />} />
99+
<Route path={FRONTEND_PATHS.SESSION_LOGGED_OUT} element={<SessionLoggedOutPage />} />
100+
<Route path={FRONTEND_PATHS.SELECT_YOUR_ROLE} element={<SelectYourRolePage />} />
101+
<Route path={FRONTEND_PATHS.YOUR_SELECTED_ROLE} element={<YourSelectedRolePage />} />
102+
<Route path={FRONTEND_PATHS.CHANGE_YOUR_ROLE} element={<ChangeRolePage />} />
103+
<Route path={FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID} element={<SearchPrescriptionPage />} />
104+
<Route path={FRONTEND_PATHS.SEARCH_BY_NHS_NUMBER} element={<SearchPrescriptionPage />} />
105+
<Route path={FRONTEND_PATHS.SEARCH_BY_BASIC_DETAILS} element={<SearchPrescriptionPage />} />
106+
<Route path={FRONTEND_PATHS.PRESCRIPTION_LIST_CURRENT} element={<PrescriptionListPage />} />
107+
<Route path={FRONTEND_PATHS.PRESCRIPTION_LIST_FUTURE} element={<PrescriptionListPage />} />
108+
<Route path={FRONTEND_PATHS.PRESCRIPTION_LIST_PAST} element={<PrescriptionListPage />} />
109+
<Route path={FRONTEND_PATHS.PRESCRIPTION_DETAILS_PAGE} element={<PrescriptionDetailsPage />} />
110+
<Route path={FRONTEND_PATHS.PATIENT_SEARCH_RESULTS} element={<SearchResultsPage />} />
111+
<Route path={FRONTEND_PATHS.TOO_MANY_SEARCH_RESULTS} element={<TooManySearchResultsPage />} />
112+
<Route path={FRONTEND_PATHS.NO_PATIENT_FOUND} element={<NoPatientsFoundPage />} />
113+
<Route path={FRONTEND_PATHS.NO_PRESCRIPTIONS_FOUND} element={<NoPrescriptionsFoundPage />} />
114+
<Route path={FRONTEND_PATHS.PRIVACY_NOTICE} element={<PrivacyNoticePage />} />
115+
</Route>
116+
</Routes>
117+
</NavigationProvider>
118+
</SearchProvider>
119+
</PrescriptionInformationProvider>
120+
</PatientDetailsProvider>
121+
</>
91122
)
92123
}
93124

‎packages/cpt-ui/src/components/EpsSpinner.tsx‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React from "react"
1+
import React, {useEffect, useRef} from "react"
22

33
import {EpsSpinnerStrings} from "@/constants/ui-strings/EpsSpinnerStrings"
44

@@ -8,6 +8,15 @@ function Spinner({
88
fraction = 0.2, // The fraction of the hoop that is green
99
speed = 1 // The speed (in seconds) for one full rotation
1010
}) {
11+
const statusRef = useRef<HTMLDivElement>(null)
12+
13+
useEffect(() => {
14+
// Focus the status element when the spinner mounts to ensure it's announced
15+
if (statusRef.current) {
16+
statusRef.current.focus()
17+
}
18+
}, [])
19+
1120
// The portion that should appear green is defined by "fraction"
1221
// If fraction = 0.25, then 25% of the hoop is green and 75% is grey
1322
const circumference = 2 * Math.PI * radius
@@ -22,9 +31,19 @@ function Spinner({
2231
alignItems: "center",
2332
justifyContent: "center"
2433
}}
25-
role="progressbar"
2634
data-testid="spinner"
2735
>
36+
{/* Screen reader announcement */}
37+
<div
38+
ref={statusRef}
39+
role="status"
40+
aria-live="assertive"
41+
aria-atomic="true"
42+
tabIndex={-1}
43+
className="nhsuk-u-visually-hidden"
44+
>
45+
{EpsSpinnerStrings.loading}
46+
</div>
2847
<div
2948
className="spinner-container"
3049
style={{

‎packages/cpt-ui/src/components/EpsTabs.tsx‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, {useEffect, useCallback} from "react"
1+
import React, {useEffect, useCallback, useRef} from "react"
22
import {Link, useNavigate} from "react-router-dom"
33
import {Tabs} from "nhsuk-react-components"
44
import "../styles/tabs.scss"
@@ -26,6 +26,9 @@ export default function EpsTabs({
2626
const tabClass = `${baseClass} ${variantClass}`.trim()
2727

2828
const navigate = useNavigate()
29+
const keyboardNavigatedRef = useRef(false)
30+
const clickNavigatedRef = useRef(false)
31+
const lastKeyboardFocusedTabRef = useRef<string | null>(null)
2932
const handleKeyDown = useCallback((event: KeyboardEvent) => {
3033
const activeElement = document.activeElement
3134

@@ -50,6 +53,7 @@ export default function EpsTabs({
5053

5154
if (newTabIndex !== currentTabIndex) {
5255
const newTab = tabs[newTabIndex]
56+
keyboardNavigatedRef.current = true
5357
navigate(newTab.link)
5458
}
5559
}, [navigate, tabHeaderArray, activeTabPath])
@@ -62,15 +66,33 @@ export default function EpsTabs({
6266
}
6367
}, [handleKeyDown])
6468

65-
// Ensure focus moves to the active tab when the route/tab changes
6669
useEffect(() => {
67-
const activeId = `tab_${activeTabPath.substring(1)}`
68-
const activeEl = document.getElementById(activeId) as HTMLAnchorElement | null
69-
if (activeEl) {
70-
activeEl.focus()
70+
if (keyboardNavigatedRef.current || clickNavigatedRef.current) {
71+
if (lastKeyboardFocusedTabRef.current) {
72+
const prevTab = document.getElementById(lastKeyboardFocusedTabRef.current)
73+
if (prevTab) {
74+
prevTab.classList.remove("keyboard-focused")
75+
}
76+
}
77+
78+
const activeId = `tab_${activeTabPath.substring(1)}`
79+
const activeEl = document.getElementById(activeId) as HTMLAnchorElement | null
80+
if (activeEl) {
81+
if (keyboardNavigatedRef.current) {
82+
activeEl.focus()
83+
}
84+
activeEl.classList.add("keyboard-focused")
85+
lastKeyboardFocusedTabRef.current = activeId
86+
}
87+
keyboardNavigatedRef.current = false
88+
clickNavigatedRef.current = false
7189
}
7290
}, [activeTabPath])
7391

92+
const handleTabClick = () => {
93+
clickNavigatedRef.current = true
94+
}
95+
7496
const renderAccessibleTitle = (title: string) => {
7597
const match = title.match(/^(.*)\s\((\d+)\)$/)
7698
if (!match) return title
@@ -113,6 +135,7 @@ export default function EpsTabs({
113135
to={tab.link}
114136
data-testid={`eps-tab-heading ${tab.link}`}
115137
tabIndex={isActive ? 0 : -1}
138+
onClick={() => handleTabClick()}
116139
>
117140
{renderAccessibleTitle(tab.title)}
118141
</Link>

‎packages/cpt-ui/src/constants/ui-strings/EpsSpinnerStrings.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
export const EpsSpinnerStrings = {
2-
loading: "Loading..."
2+
loading: "Loading"
33
}

0 commit comments

Comments
 (0)