fix: update audit log back link text and fix date filter row layout - #670
Conversation
Fixes two formatting inconsistencies in the Audit Log Viewer: - Back link on audit-log-detail changed from "Back to audit log list" to "Back" (en/cy) - Removed CSS rule forcing Year date input onto a new row in the filter panel Closes #467 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughTwo audit-log UI fixes are applied: the detail page back link text is shortened to "Back"/"Yn ôl" via localisation strings, and the list page date-filter layout is corrected by removing a SCSS padding rule and reorganising the template under a new ChangesAudit Log Formatting Fixes
Package Exports and Sonar Config
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 420f90bb-e94b-487c-ab8c-bc892f110a13
📒 Files selected for processing (8)
docs/tickets/467/plan.mddocs/tickets/467/review.mddocs/tickets/467/tasks.mddocs/tickets/467/ticket.mdlibs/system-admin-pages/src/assets/css/dashboard.scsslibs/system-admin-pages/src/pages/audit-log-detail/cy.tslibs/system-admin-pages/src/pages/audit-log-detail/en.tslibs/system-admin-pages/src/pages/audit-log-list/index.njk
💤 Files with no reviewable changes (2)
- libs/system-admin-pages/src/pages/audit-log-list/index.njk
- libs/system-admin-pages/src/assets/css/dashboard.scss
| The test at lines 69–105 ("should render audit log detail when found") asserts: | ||
| ```typescript | ||
| backToListText: expect.any(String), | ||
| ``` | ||
| This assertion continues to pass after the change because the value is still a string. No test update is required unless the team wants to assert the exact string value. The existing assertion is sufficient for the scope of this ticket. | ||
|
|
There was a problem hiding this comment.
Align test guidance with the ticket acceptance checks.
Line 67 says no test update is required, but the ticket/PR test plan expects validating exact back link text in English and Welsh. Please make the plan explicit and consistent to avoid conflicting implementation guidance.
🎭 Playwright E2E Test Results84 tests 52 ✅ 5m 59s ⏱️ Results for commit 120ca39. ♻️ This comment has been updated with latest results. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
The govuk-date-input flex container wraps by default, causing the Year input to drop onto a second row in the one-quarter filter panel. Using flex-wrap: nowrap keeps all three inputs on a single row. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 86d910bc-a658-473d-85cd-218e321f2095
📒 Files selected for processing (1)
libs/system-admin-pages/src/assets/css/dashboard.scss
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
…erflow Overriding block content directly causes govuk-grid-row negative margins to bleed outside govuk-main-wrapper. Using page_content places content inside the already-padded width container, matching all other pages. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
govuk-grid-row uses negative margins (-15px each side) which bleed outside govuk-main-wrapper. Replaced with a flexbox layout that has no negative margins and stays within the width container. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
The .js extension caused Vite to fail resolving the file since it only exists as .ts in the source tree. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The wildcard export pattern does not map .js paths to .ts source files. Add explicit named exports matching the pattern used by other packages so Vite resolves the imports correctly in dev mode. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
- Add VITE_DEV_BUILD flag to suppress content hashes from dev asset filenames, preventing stale references after rebuilds - Reintroduce govuk-grid-row on the audit-log layout wrapper so GOV.UK grid columns work correctly alongside the flexbox container - Remove gap between filter and content panels; strip right padding from the three-quarters column to avoid overflow - Make the 'view' column header visually hidden so its narrow width doesn't push other columns; use word-break on email cells to prevent overflow - Enable overflow-x: auto on the content area as a safety net for narrow viewports Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0aba461a-4f05-4ab4-b8b7-8486ab7a0527
📒 Files selected for processing (5)
apps/web/package.jsonapps/web/vite.build.tslibs/list-types/common/package.jsonlibs/system-admin-pages/src/assets/css/dashboard.scsslibs/system-admin-pages/src/pages/audit-log-list/index.njk
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/system-admin-pages/src/pages/audit-log-list/index.njk
| .audit-log-layout__word-break { | ||
| word-break: break-word; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is word-break: break-worddeprecated in current CSS specs, and isoverflow-wrap: anywhere the recommended replacement?
💡 Result:
Yes, the word-break: break-word value is deprecated in current CSS specifications [1][2]. It was historically used as a legacy alias to prevent text from overflowing its container, but it is no longer considered appropriate for new development [3][4]. The recommended replacement is overflow-wrap: anywhere [2][4]. While both word-break: break-word and overflow-wrap: anywhere are designed to prevent long, unbreakable strings (like long words or URLs) from overflowing their containers, overflow-wrap: anywhere is the modern, standards-compliant way to achieve this behavior [4]. Using overflow-wrap: anywhere is preferred because it correctly accounts for soft wrap opportunities when calculating min-content intrinsic sizes, which helps ensure elements like inline-blocks or flex items behave predictably [4][5]. In contrast, the deprecated word-break: break-word acts as an alias for word-break: normal combined with overflow-wrap: anywhere, but it lacks the consistent, standardized behavior defined by modern CSS Text specifications [1][6][4].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/word-break
- 2: https://web-platform-dx.github.io/web-features-explorer/features/word-break-break-word/
- 3: [css-text-3] Normatively disallow break-word w3c/csswg-drafts#4087
- 4: https://deku.posstree.com/en/web/css-overflow-wrap/
- 5: https://github.com/mdn/content/blob/main/files/en-us/web/css/reference/properties/overflow-wrap/index.md
- 6: https://stackoverflow.com/questions/56618668/do-overflow-wrap-break-word-and-word-break-break-word-ever-behave-differen
Replace word-break: break-word with overflow-wrap.
word-break: break-word is a legacy/deprecated value; use overflow-wrap instead for consistent cross-engine wrapping.
Suggested patch
.audit-log-layout__word-break {
- word-break: break-word;
+ overflow-wrap: anywhere;
+ word-break: normal;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .audit-log-layout__word-break { | |
| word-break: break-word; | |
| } | |
| .audit-log-layout__word-break { | |
| overflow-wrap: anywhere; | |
| word-break: normal; | |
| } |
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 63-63: Deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
The Vite manifest already handles hashed filename resolution at runtime, making the dev-build flag and custom output naming redundant. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Newer vitest hangs on dynamic imports of non-existent workspace packages rather than throwing. Mock the missing @hmcts/civil-daily-cause-list as an empty module so the test resolves synchronously via the "no validate function" path. Unify both no-schema error messages to "No JSON schema available" for consistency. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Resolve conflict in list-type-validator.test.ts - use master's cleaner comment wording for the civil-daily-cause-list mock. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
parseInt("27er", 10) silently returns 27, so inputs like "27er", "3er",
and "2007er" passed validation and produced a valid date with no error.
Replace parseInt with a /^\d+$/ guard and Number() — any non-digit
character in any field now returns null immediately.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
|
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
10 STATUS + IMPL changes (closed issue + merged closing PR → verified): REQ-0078 (#301): implemented → verified (PR #458) REQ-0105 (#428): in_progress → verified (PR #749) REQ-0106 (#429): approved → verified (PR #761) REQ-0107 (#431): implemented → verified (PR #701) REQ-0108 (#434): approved → verified (PR #772) REQ-0109 (#436): implemented → verified (PR #727) REQ-0112 (#467): implemented → verified (PR #670) REQ-0124 (#563): approved → verified (PR #782) REQ-0135 (#569): in_progress → verified (PR #748) REQ-0137 (#729): approved → verified (PR #766) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3 participants
Summary
.app-date-filter-narrow .govuk-date-input__item:last-child { padding-top: 10px; }that was forcing the Year date input onto a second rowapp-date-filter-narrowclass from the date inputformGroupin audit-log-list templateTest plan
yarn test— all 637 tests pass/audit-log-detail?id=<id>and confirm back link reads "Back"?lng=cyand confirm back link reads "Yn ôl"/audit-log-list, open filter panel, confirm Day/Month/Year date inputs appear on one rowCloses #467
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Chores