Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/language/texts/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export function en() {
'form_filler.file_uploader_delete_warning': 'Are you sure you want to delete this attachment?',
'form_filler.file_uploader_delete_button_confirm': 'Yes, delete attachment',
'form_filler.file_uploader_list_header_file_size': 'File size',
'form_filler.file_uploader_list_header_thumbnail': 'Preview',
'form_filler.file_uploader_list_header_name': 'Name',
'form_filler.file_uploader_list_header_status': 'Status',
'form_filler.file_uploader_list_header_delete_sr': 'Delete',
Expand Down
1 change: 1 addition & 0 deletions src/language/texts/nb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export function nb() {
'form_filler.file_uploader_delete_warning': 'Er du sikker på at du vil slette dette vedlegget?',
'form_filler.file_uploader_delete_button_confirm': 'Ja, slett vedlegg',
'form_filler.file_uploader_list_header_file_size': 'Filstørrelse',
'form_filler.file_uploader_list_header_thumbnail': 'Forhåndsvisning',
'form_filler.file_uploader_list_header_name': 'Navn',
'form_filler.file_uploader_list_header_status': 'Status',
'form_filler.file_uploader_list_status_done': 'Ferdig lastet',
Expand Down
1 change: 1 addition & 0 deletions src/language/texts/nn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export function nn() {
'form_filler.file_uploader_delete_warning': 'Er du sikker på at du vil sletta dette vedlegget?',
'form_filler.file_uploader_delete_button_confirm': 'Ja, slett vedlegg',
'form_filler.file_uploader_list_header_file_size': 'Filstorleik',
'form_filler.file_uploader_list_header_thumbnail': 'Førehandsvisning',
'form_filler.file_uploader_list_header_name': 'Namn',
'form_filler.file_uploader_list_header_status': 'Status',
'form_filler.file_uploader_list_status_done': 'Ferdig lasta',
Expand Down
132 changes: 132 additions & 0 deletions src/layout/FileUpload/FileUploadTable/AttachmentThumbnail.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
.thumbnailContainer {
margin-top: 0.5rem;
margin-bottom: 0.5rem;
display: flex;
align-items: center;
}

.thumbnail {
max-width: 80px;
max-height: 56px;
object-fit: contain;
}

.thumbnailMobile {
max-width: 64px;
max-height: 48px;
object-fit: contain;
}

.previewBackdrop {
position: fixed;
top: 0;
left: 0;
right: 0;
bottom: 0;
background-color: rgba(0, 0, 0, 0.7);
display: flex;
align-items: center;
justify-content: center;
z-index: 1000;
animation: fadeIn 0.3s ease-in-out;
}

.previewModal {
position: relative;
background-color: #fff;
border-radius: 8px;
box-shadow: 0 4px 20px rgba(0, 0, 0, 0.3);
max-width: 90vw;
max-height: 90vh;
overflow: auto;
padding: 0;
display: flex;
flex-direction: column;
}

.previewHeader {
position: relative;
display: flex;
justify-content: space-between;
align-items: center;
padding: 16px 20px;
background-color: #fff;
border-radius: 8px 8px 0 0;
gap: 16px;
}

.previewHeaderMobile {
position: relative;
display: flex;
justify-content: space-between;
align-items: center;
padding: 8px 20px;
background-color: #fff;
border-radius: 8px 8px 0 0;
gap: 8px;
}

.fileName {
font-weight: 500;
color: #333;
flex: 1;
word-break: break-word;
font-size: 0.95rem;
}

.previewImage {
max-width: 100%;
max-height: calc(90vh - 100px);
object-fit: contain;
display: block;
padding: 0 20px 20px 20px;
background-color: #fff;
}

.previewImageMobile {
max-width: 100%;
max-height: calc(90vh - 100px);
object-fit: contain;
display: block;
padding: 0 10px 10px 10px;
background-color: #fff;
}

.closeButton {
position: relative;
background-color: transparent;
border: none;
font-size: 32px;
cursor: pointer;
color: var(--ds-color-text-subtle);
width: 40px;
height: 40px;
display: flex;
align-items: center;
justify-content: center;
border-radius: 4px;
transition: background-color 0.2s ease;
flex-shrink: 0;
padding: 0;
}

.closeButton:hover {
color: var(--ds-color-text-default);
background-color: var(--ds-color-surface-tinted);
}

.imageLoading {
display: flex;
justify-content: center;
align-items: center;
min-height: 200px;
}

@keyframes fadeIn {
from {
opacity: 0;
}
to {
opacity: 1;
}
}
59 changes: 59 additions & 0 deletions src/layout/FileUpload/FileUploadTable/AttachmentThumbnail.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import React from 'react';

import { isAttachmentUploaded } from 'src/features/attachments';
import { useInstanceDataElements, useLaxInstanceId } from 'src/features/instance/InstanceContext';
import { useCurrentLanguage } from 'src/features/language/LanguageProvider';
import classes from 'src/layout/FileUpload/FileUploadTable/AttachmentThumbnail.module.css';
import { getDataElementUrl } from 'src/utils/urls/appUrlHelper';
import { makeUrlRelativeIfSameDomain } from 'src/utils/urls/urlHelper';
import type { IAttachment } from 'src/features/attachments';

interface IAttachmentThumbnailProps {
attachment: IAttachment;
mobileView: boolean;
onThumbnailClick?: () => void;
}

export const AttachmentThumbnail = ({ attachment, mobileView, onThumbnailClick }: IAttachmentThumbnailProps) => {
const dataElements = useInstanceDataElements(undefined);
const instanceId = useLaxInstanceId();
const language = useCurrentLanguage();

// Only uploaded attachments can have thumbnails
if (!instanceId || !isAttachmentUploaded(attachment)) {
return '';
}

const thumbnailLink = attachment.data.metadata?.find((meta) => meta.key === 'thumbnailLink')?.value;
if (!thumbnailLink) {
return '';
}

const thumbnailDataElement = dataElements.find(
(el) =>
el.dataType === 'thumbnail' &&
el.metadata?.some((meta) => meta.key === 'attachmentLink' && meta.value === thumbnailLink),
);

const url = thumbnailDataElement
? makeUrlRelativeIfSameDomain(getDataElementUrl(instanceId, thumbnailDataElement.id, language))
: undefined;

return (
<div
className={classes.thumbnailContainer}
data-testid='attachment-thumbnail'
onClick={onThumbnailClick}
style={onThumbnailClick ? { cursor: 'pointer' } : {}}
role={onThumbnailClick ? 'button' : undefined}
tabIndex={onThumbnailClick ? 0 : undefined}
onKeyDown={onThumbnailClick ? (e) => e.key === 'Enter' && onThumbnailClick() : undefined}
>
<img
src={url}
alt={`Thumbnail for ${attachment.data.filename}`}
className={mobileView ? classes.thumbnailMobile : classes.thumbnail}
/>
</div>
);
Comment on lines +42 to +58
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Improve accessibility and add safety check for image URL.

Two concerns:

  1. Missing URL validation: The url can be undefined if no thumbnailDataElement is found, yet it's passed directly to the img src attribute without validation. This could cause a broken image display.

  2. Accessibility pattern: Using a div with button-like behavior (role, tabIndex, onClick, onKeyDown) is non-standard. While functional, a native <button> element provides better accessibility and keyboard navigation out of the box.

🔎 Proposed fixes

Add URL validation:

   const url = thumbnailDataElement
     ? makeUrlRelativeIfSameDomain(getDataElementUrl(instanceId, thumbnailDataElement.id, language))
     : undefined;

+  if (!url) {
+    return null;
+  }
+
   return (

Alternative: Use a button wrapper for better accessibility (optional, if styling allows):

   return (
-    <div
+    <button
+      type="button"
       className={classes.thumbnailContainer}
       data-testid='attachment-thumbnail'
       onClick={onThumbnailClick}
-      style={onThumbnailClick ? { cursor: 'pointer' } : {}}
-      role={onThumbnailClick ? 'button' : undefined}
-      tabIndex={onThumbnailClick ? 0 : undefined}
-      onKeyDown={onThumbnailClick ? (e) => e.key === 'Enter' && onThumbnailClick() : undefined}
+      disabled={!onThumbnailClick}
     >
       <img
         src={url}
         alt={`Thumbnail for ${attachment.data.filename}`}
         className={mobileView ? classes.thumbnailMobile : classes.thumbnail}
       />
-    </div>
+    </button>
   );

Note: If using a button, you may need to add CSS to remove default button styling.

📝 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.

Suggested change
return (
<div
className={classes.thumbnailContainer}
data-testid='attachment-thumbnail'
onClick={onThumbnailClick}
style={onThumbnailClick ? { cursor: 'pointer' } : {}}
role={onThumbnailClick ? 'button' : undefined}
tabIndex={onThumbnailClick ? 0 : undefined}
onKeyDown={onThumbnailClick ? (e) => e.key === 'Enter' && onThumbnailClick() : undefined}
>
<img
src={url}
alt={`Thumbnail for ${attachment.data.filename}`}
className={mobileView ? classes.thumbnailMobile : classes.thumbnail}
/>
</div>
);
return (
<button
type="button"
className={classes.thumbnailContainer}
data-testid='attachment-thumbnail'
onClick={onThumbnailClick}
disabled={!onThumbnailClick}
>
<img
src={url}
alt={`Thumbnail for ${attachment.data.filename}`}
className={mobileView ? classes.thumbnailMobile : classes.thumbnail}
/>
</button>
);
🤖 Prompt for AI Agents
In @src/layout/FileUpload/FileUploadTable/AttachmentThumbnail.tsx around lines
42-58, AttachmentThumbnail renders an <img> with src={url} where url can be
undefined and wraps it in a div faking button behavior; fix by (1) guarding the
image src: if url is falsy, render a placeholder image or omit the <img>
entirely and/or add an alt that indicates no preview available (update the usage
of the url variable and the <img> element in AttachmentThumbnail), and (2)
replace the interactive div (classes.thumbnailContainer with
role/tabIndex/onKeyDown) with a native <button> when onThumbnailClick is
provided (or render a non-interactive container when not), wiring onClick to
onThumbnailClick and preserving classes.thumbnailContainer and
mobileView-dependent class names while removing custom keyboard handlers so
native button accessibility is used.

};
Loading
Loading