Fix: [AEA-5884] - Part 2: Fix disabling button on press & prevent cross firing #1884
Fix: [AEA-5884] - Part 2: Fix disabling button on press & prevent cross firing #1884connoravo-nhs merged 35 commits intomainfrom
Conversation
|
This PR is linked to a ticket in an NHS Digital JIRA Project. Here's a handy link to the ticket: AEA-5884 |
…cted-role and tracker-user-info backend calls
| remainingMinutes: Math.floor(remainingTime / 60000), | ||
| twoMinuteThreshold: twoMinutes | ||
| }) | ||
| const twoMinutes = 14 * 60 * 1000 |
There was a problem hiding this comment.
To be reset after Tom's tested
| expect(liveRegion).toHaveTextContent("You will be logged out in 1 second.") | ||
| }) | ||
|
|
||
| it("does not announce at non-specified intervals", () => { |
There was a problem hiding this comment.
Should we assert on the call to shouldAnnounceAtTime to ensure it's false for these scenarios?
| expect(liveRegion).toHaveTextContent("You will be logged out in 4 minutes and 15 seconds.") | ||
| }) | ||
|
|
||
| it("announces at specific intervals when time is 20 seconds or less", () => { |
There was a problem hiding this comment.
Should we assert on shouldAnnounceAtTime returning true here?
| })) | ||
|
|
||
| const createMockAuth = (overrides = {}): AuthContextType => ({ | ||
| error: null, |
There was a problem hiding this comment.
Use the MockAuthState here please. Trying to consolidate all uses so that it's change once, update many when the auth context changes
…licate firing events and clean up of function location Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com>
Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com>
…roperly reset Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com>
Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com>
Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com>
Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com>
Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent duplicate/cross-triggered logout/session-timeout actions by centralising modal state in AuthProvider, disabling action buttons during in-flight actions, and improving the session-timeout modal UX/accessibility.
Changes:
- Move session-timeout modal state into
AuthProvider(persisted viauseLocalStorageState) and refactoruseSessionTimeoutto use auth context directly. - Update logout and session-timeout modals/components to support button disabling and improved announcements.
- Add/adjust tests to cover the new modal behaviours and helper logic.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cpt-ui/src/styles/EpsModal.scss | Set fixed/modal widths and responsive sizing tweaks. |
| packages/cpt-ui/src/pages/LogoutPage.tsx | Avoid repeated sign-out calls; reset isSigningOut after completion. |
| packages/cpt-ui/src/hooks/useSessionTimeout.ts | Refactor to auth-context-driven handlers with action locking. |
| packages/cpt-ui/src/helpers/logout.tsx | Add setStateForSignOut and error handling around Cognito sign-out. |
| packages/cpt-ui/src/context/AuthProvider.tsx | Add persisted modal state (sessionTimeoutModalInfo, logoutModalType) and setters to context. |
| packages/cpt-ui/src/context/AccessProvider.tsx | Populate session timeout modal state via auth context; remove old AccessContext API surface. |
| packages/cpt-ui/src/constants/ui-strings/YourSelectedRoleStrings.ts | Fix string casing (“details”). |
| packages/cpt-ui/src/constants/ui-strings/SessionTimeoutModalStrings.ts | Update session-timeout title/message copy. |
| packages/cpt-ui/src/components/SessionTimeoutModal.tsx | Add aria-live announcements, countdown handling, and disabled button state plumbing. |
| packages/cpt-ui/src/components/EpsModal.tsx | Add aria-describedby support. |
| packages/cpt-ui/src/components/EpsLogoutModal.tsx | Add buttonDisabled prop to disable confirm action. |
| packages/cpt-ui/src/components/EpsHeader.tsx | Use logoutModalType and disable confirm button to prevent repeated sign-out. |
| packages/cpt-ui/src/App.tsx | Render session timeout modal based on auth modal state; use refactored hook. |
| packages/cpt-ui/tests/useSessionTimeout.test.tsx | Rewrite tests for new auth-context-based hook behaviours. |
| packages/cpt-ui/tests/mocks/AuthStateMock.tsx | Extend auth mock with new modal state and setters. |
| packages/cpt-ui/tests/logout.test.tsx | Add test coverage for logout helpers and restart-login behaviour. |
| packages/cpt-ui/tests/SessionTimeoutModal.test.tsx | Add comprehensive tests for modal rendering, focus, countdown, and announcements. |
| packages/cpt-ui/tests/SearchPrescriptionPagePaths.test.tsx | Update provider wiring for simplified AccessContext. |
| packages/cpt-ui/tests/SearchPrescriptionPageKeyboard.test.tsx | Update provider wiring for simplified AccessContext. |
| packages/cpt-ui/tests/SearchPrescriptionPageInputDetection.test.tsx | Update provider wiring for simplified AccessContext. |
| packages/cpt-ui/tests/SearchPrescriptionPageCoverage.test.tsx | Update provider wiring for simplified AccessContext. |
| packages/cpt-ui/tests/EpsLogoutModal.test.tsx | Add tests for confirm-button disabling behaviour. |
| packages/cpt-ui/tests/EpsHeader.test.tsx | Adjust test provider setup due to AccessContext changes. |
| packages/cpt-ui/tests/App.test.tsx | Update mocks to align with auth-state-driven modal rendering. |
| packages/cpt-ui/tests/AccessProvider.test.tsx | Update tests for session-timeout handling via auth modal state. |
| packages/common/commonTypes/src/sessionTimeoutModalState.ts | Introduce shared SessionTimeoutModal type. |
| packages/common/commonTypes/src/index.ts | Export SessionTimeoutModal type from common-types index. |
Comments suppressed due to low confidence (1)
packages/cpt-ui/src/context/AccessProvider.tsx:118
const twoMinutes = 14 * 60 * 1000contradicts both the variable name and the comment below indicating a 2-minute threshold. If the intent is a 2-minute warning, this should remain2 * 60 * 1000(or rename/update comments/tests if 14 minutes is actually intended).
const twoMinutes = 14 * 60 * 1000 // Minutes into milliseconds
if (remainingTime <= twoMinutes && remainingTime > 0) {
// Show timeout modal when 2 minutes or less remaining
logger.info("Session timeout warning triggered - showing modal", {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Manipulate lock rather than idempotent logout Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com>
…l/eps-prescription-tracker-ui into aea-5884-comments-on-modal
…cause multi-tab issues Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com>
…-parser Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com>
Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com>
|



Summary
Details
Fix disabling button on press & prevent cross firing
Disable logout button on user initiated logout modal after it's been clicked