feat(compliance): image gallery for case attachments (#1168) - #1182
Merged
Conversation
Turns the Rapport Billeder cell from a static count into a button that opens a gallery over the case's attachments. Extends the existing CalendarImageLightboxComponent rather than adding a second lightbox: it already spoke the exact filename and bearer-auth contract this page needs. It is now exported from CalendarModule, which ComplianceReportModule already imported, so nothing had to move. It gains an optional header (case id, title, property), a caption, an aria-live counter, a 64x64 thumbnail strip and arrow-key navigation. The server now emits the _300_ derivative name alongside the _700_ one, so the thumbnail strip loads thumbnails instead of full-resolution images. Deriving _300_ on the client was rejected: filenames are the server's business, and the client only ever chooses between two server-supplied strings. Opening an n-image gallery therefore costs n thumbnails plus one full-size image. The strip is gated on the caller actually supplying thumbnail names, not merely on having several images. Without that gate the historical task card -- which supplies none -- would have rendered a strip whose sources fall back to full-size ORIGINAL uploads, costing n extra authenticated fetches and n more base64 strings on a caller that asked for none of this. With exactly one image the previous, next, counter and strip are all absent from the DOM. The counter used to render unconditionally. The prototype's cancel-event interception and src-attribute clearing are deliberately not ported: MatDialog closes on Escape and backdrop click, and the component is constructed per open and destroyed on close, so there is no residual index and no stale src. Also fixes an accessibility defect found while theming: the active thumbnail outline cannot use --md-primary, which measures 2.58:1 against the lightbox surface under theme-workspace light -- below the 3:1 floor for a non-text indicator, and passing in the other three theme/mode combinations, so it would have shipped unnoticed. i18n: six new keys in all 26 locales. They were authored per locale rather than machine-translated, because the existing Close key is MT junk for the adjective "near" in nine languages (Blizko, Cerca, Vicino, Naera, Dichtbij, ...), which is tracked separately in #1181. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LcMeVFxqWqQzvqHjAa3Xkc
There was a problem hiding this comment.
🟡 Changes recommended
The updated Angular unit spec setup will fail DI because Overlay is now required by the component but isn’t provided in the TestBed configurations (and the new Playwright spec introduces a fixed sleep and bypasses the shared wait helpers).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an image gallery experience for Compliance Report case attachments by reusing and extending the existing calendar lightbox, while also having the backend emit _300_ thumbnail derivative names to avoid loading full-size images for the thumbnail strip.
Changes:
- Backend: include
_300_thumbnail derivative names alongside_700_image names in compliance report image DTOs. - Frontend: make the Compliance Report “Billeder” cell open the shared
CalendarImageLightboxComponent, including header/caption/counter/thumb-strip/keyboard navigation behavior. - Tests/i18n: add unit coverage for image-name mapping/alignment rules, add a Playwright spec, and propagate new i18n keys across locales.
File summaries
| File | Description |
|---|---|
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Services/BackendConfigurationComplianceReportService/IBackendConfigurationComplianceReportService.cs | Updates service contract docs to mention _300_ thumbnails being emitted. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Services/BackendConfigurationComplianceReportService/ComplianceReportEformProjector.cs | Emits ThumbnailFileName (_300_) alongside FileName (_700_) when projecting images. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Infrastructure/Models/ComplianceReport/ComplianceReportImageModel.cs | Adds ThumbnailFileName to the backend image model with derivation documentation. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/compliance-report/components/compliance-report-view/compliance-report-view.component.ts | Builds renderable image/thumbnail name arrays and opens the shared lightbox from the images cell. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/compliance-report/components/compliance-report-view/compliance-report-view.component.spec.ts | Adds unit tests for image name filtering/alignment and gallery gating behavior. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/compliance-report/components/compliance-report-view/compliance-report-view.component.scss | Styles the interactive images cell as a non-jittering button with accessible focus ring. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/compliance-report/components/compliance-report-view/compliance-report-view.component.html | Renders the images cell as either static count or a button that opens the gallery. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/calendar/modals/task-preview-modal/task-preview-modal.component.ts | Updates calendar task card lightbox open config (panel class, documents shared behavior changes). |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/calendar/modals/calendar-image-lightbox/calendar-image-lightbox.component.ts | Extends the lightbox with optional header, caption, counter, thumb strip, and keyboard navigation. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/calendar/modals/calendar-image-lightbox/calendar-image-lightbox.component.scss | Re-themes lightbox styles, adds thumb strip and responsive full-bleed layout rules. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/calendar/modals/calendar-image-lightbox/calendar-image-lightbox.component.html | Adds header/caption/counter/thumb-strip markup while preserving existing e2e IDs. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/calendar/calendar.module.ts | Re-exports the lightbox component so other modules (Compliance report) can open it. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/models/compliance-report/compliance-report.model.ts | Adds thumbnailFileName to the frontend compliance report image model. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/ukUA.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/svSE.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/slSL.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/skSK.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/roRO.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/ptPT.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/ptBR.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/plPL.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/noNO.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/nlNL.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/lvLV.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/ltLT.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/itIT.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/isIS.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/huHU.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/hrHR.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/frFR.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/fiFI.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/etET.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/esES.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/enUS.ts | Adds gallery-related i18n keys and documents their intent. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/elGR.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/deDE.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/da.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/csCZ.ts | Adds gallery-related i18n keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/bgBG.ts | Adds gallery-related i18n keys. |
| eform-client/playwright/e2e/plugins/backend-configuration-pn/s/compliance-image-gallery.spec.ts | Adds Playwright coverage for the compliance report’s gallery open/UX behavior (conditionally data-dependent). |
Review details
- Files reviewed: 40/40 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+431
to
+435
| {provide: BackendConfigurationPnPropertiesService, useValue: {getAllPropertiesDictionary: jest.fn()}}, | ||
| {provide: BackendConfigurationPnCalendarService, useValue: {getBoards: jest.fn()}}, | ||
| {provide: MatDialog, useValue: {open: jest.fn()}}, | ||
| {provide: Router, useValue: {navigate: jest.fn(), url: '/x'}}, | ||
| ], |
Comment on lines
+68
to
+74
| async function fetchReport(page: Page): Promise<void> { | ||
| const response = page.waitForResponse( | ||
| (r) => r.url().includes('/compliance-report/eform-columns'), | ||
| { timeout: 60000 }, | ||
| ); | ||
| await page.locator('#complianceShowReportBtn').click(); | ||
| await response; |
Comment on lines
+58
to
+66
| async function goToRapport(page: Page): Promise<void> { | ||
| await page.goto(BASE_URL); | ||
| await new LoginPage(page).login(); | ||
| await page.waitForTimeout(2000); | ||
| await page.goto(PAGE_URL); | ||
| await page.locator('#complianceFilterProperty').waitFor({ state: 'visible', timeout: 60000 }); | ||
| await page.locator('#complianceMode-report').click(); | ||
| await expect(page.locator('#complianceMode-report')).toHaveAttribute('aria-pressed', 'true'); | ||
| } |
16 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1168. Part of the compliance-report epic #1160. Completes the acceptance criterion #1167 deliberately left unmet — the
Billedercell is now a button that opens a gallery.Approach
Extended
CalendarImageLightboxComponentrather than adding a second lightbox. It already spoke the exact filename and bearer-auth contract this page needs. It is now exported fromCalendarModule, whichComplianceReportModulealready imported, so nothing moved. It gains an optional header (case id, title, property), a caption, anaria-livecounter, a 64×64 thumbnail strip and arrow-key navigation.The server now emits the
_300_derivative name alongside_700_, so the strip loads thumbnails rather than full-resolution images. Deriving_300_client-side was rejected — filenames are the server's business, and the client only ever picks between two server-supplied strings. An n-image gallery costs n thumbnails + 1 full-size image.The strip is gated on the caller supplying thumbnail names, not merely on having several images. Without that gate the historical task card — which supplies none — would render a strip falling back to full-size original uploads: n extra authenticated fetches and n more base64 strings, on a caller that asked for none of it.
What changed for the calendar task card
The thumb strip is the one thing restored to its previous behaviour. The rest is a deliberate shared improvement, and the code comments now enumerate it rather than claiming an identity that
git show stable:refutes:count > 0, so also at n=1);Close→Close gallery;altwent from""to a real description;.lightbox*→.calendar-lightbox__*, and colour literals replaced by theme tokens;panelClass, which brings full-bleed layout below 720px.Accessibility fix found while theming
The active thumbnail outline cannot use
--md-primary: it measures 2.58:1 against the lightbox surface undertheme-workspacelight — below the 3:1 floor of WCAG 1.4.11 for a non-text indicator — while passing in the other three theme/mode combinations (7.60, 9.59, 4.21). It would have shipped unnoticed.--text-whiteis used instead (15.03–16.48:1). All ten ratios in this PR were re-derived independently by both review gates.Deliberately not ported
The prototype's
cancel-event interception andsrc-attribute clearing. MatDialog closes on Escape and backdrop click, and the component is constructed per open and destroyed on close, so there is no residual index and no stalesrcto clear.i18n
Six new keys in all 26 locales, +6 lines per file, zero deletions, placeholder tokens verified intact across all 144 values.
They were authored per locale rather than machine-translated. The existing
Closekey is MT junk for the adjective "near" in nine languages (Blízko,Cerca,Vicino,Nära,Dichtbij,Κοντά, …), so every dialog close button in those languages reads "Near" — tracked separately in #1181, along with ~28 entries whose{{placeholder}}names were themselves translated.Tests
11 new TestBed cases pin the image mapping,
canOpenGalleryand the label rules — in particular the index-alignment invariant the thumbnail design rests on (an image with no derivable name is dropped from both arrays; one with no thumbnail name is kept with a null in that slot). Alignment is asserted as a relation, never by restating the fixture. 4 Playwright tests.Coverage caveat, stated plainly: shard
sseeds no SQL, so the report renders no rows and the data-dependent Playwright tests skip. The unit cases are what actually guard this feature in CI. The skips were reworked so a missing thumb strip now fails rather than skipping the test that would have caught it.Known issues not fixed here
get-imageperforms no per-case authorization — any authenticated user can fetch any case image whose derived filename they obtain. Pre-existing and app-wide; filed as get-image performs no per-case authorization — any authenticated user can fetch any case image eform-angular-frontend#8035. This page increases exposure, which is why it is called out rather than left silent.authImagepipe has no cache and cannot be cancelled, so revisiting an image re-downloads it and closing mid-load leaves thumbnail requests in flight. Pre-existing, amplified n-fold by the strip.role="tab"withoutaria-controls, atabpanel, or rovingtabindex. Meets the issue's criterion as written; noted because the role promises more than the DOM provides.Review
Two independent gates, neither finding a blocking defect. Gate 1 caught the calendar caller's full-resolution strip regression and two spec weaknesses (a skip keyed off the element under test; an assertion reducing to
0 === 0). Gate 2 caught the false DOM-identity comments and the absent unit coverage, and corrected gate 1 on where the CDK keyboard dispatcher listens (body, notdocument). Several reviewer claims were checked, found wrong, and rejected rather than applied.🤖 Generated with Claude Code
https://claude.ai/code/session_01LcMeVFxqWqQzvqHjAa3Xkc