-
Notifications
You must be signed in to change notification settings - Fork 31
Feat/image-upload-component #3654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
fda2780
6d64978
aefafc0
35af5c2
7f3fe76
daf3f37
12e2bca
f78e84d
530b1a1
7e6c115
741579f
d15cc89
211edd7
6602e9c
a82fe47
c336050
658e9fd
bcaf56f
6b65f5c
420fc34
38a6ada
a115242
1629e7f
0d8477f
7d33561
821e4d1
805df2f
24da4ac
5976437
fbb9514
38db36c
b57f83c
acedcd9
2484b05
abb0710
97d9977
ed86243
26b53c9
1681b0a
c0c0cb8
0e685d9
cfd3a11
ee0aab3
76e1893
1f948f4
be47527
753184f
e9aa62a
f7958fe
0a02587
6f79243
ae76f44
99a3dc6
0c15edf
825d32c
edf4926
4065306
715a1b4
4afbe86
dc21ba9
d57d892
b6e0b1e
e683284
8d6ac3d
3823a85
0294604
51d8654
221082c
20c479c
019ea8f
d0993c6
8e526e7
5be3187
057189a
5f0dbcf
1d068cd
50f6b68
089711e
1b4d5ee
f391958
8812b14
f87e83b
bea697c
21b54b6
6f05f4f
2ba154b
07bce70
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,5 @@ | ||
.mediaCard { | ||
padding: 0; | ||
margin-bottom: -7px; | ||
} | ||
.mediaCard img { | ||
object-fit: cover; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
{ | ||
"name": "Display value of FileUpload component", | ||
"expression": [ | ||
"displayValue", | ||
"image" | ||
], | ||
"context": { | ||
"component": "image", | ||
"currentLayout": "Page" | ||
}, | ||
"expects": "my-image.jpg", | ||
"layouts": { | ||
"Page": { | ||
"$schema": "https://altinncdn.no/schemas/json/layout/layout.schema.v1.json", | ||
"data": { | ||
"layout": [ | ||
{ | ||
"id": "image", | ||
"type": "ImageUpload" | ||
} | ||
] | ||
} | ||
} | ||
}, | ||
"instanceDataElements": [ | ||
{ | ||
"id": "bb2b2222-2b22-2b22-222b-222222222222", | ||
"instanceGuid": "aa1a1111-1a11-1a11-111a-111111111111", | ||
"dataType": "image", | ||
"filename": "my-image.jpg", | ||
"contentType": "image/jpeg", | ||
"blobStoragePath": "", | ||
"size": 100, | ||
"locked": false, | ||
"refs": [], | ||
"created": "2021-01-01T00:00:00.000Z", | ||
"createdBy": "testUser", | ||
"lastChanged": "2021-01-01T00:00:00.000Z", | ||
"lastChangedBy": "testUser" | ||
} | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -208,6 +208,17 @@ export function en() { | |
'iframe_component.unsupported_browser_title': 'Your browser is unsupported', | ||
'iframe_component.unsupported_browser': | ||
'Your browser does not support iframes that use srcdoc. This may result in not being able to see all the content intended to be displayed here. We recommend trying a different browser.', | ||
'image_upload_component.animated_warning': 'If the image is animated, only the first frame will be shown.', | ||
'image_upload_component.button_change': 'Change image', | ||
'image_upload_component.button_delete': 'Delete image', | ||
'image_upload_component.button_save': 'Save image', | ||
'image_upload_component.crop_area': 'Crop area', | ||
'image_upload_component.slider_zoom': 'Zoom', | ||
'image_upload_component.summary_empty': "You haven't uploaded an image", | ||
'image_upload_component.reset': 'Reset position and zoom', | ||
'image_upload_component.error_invalid_file_type': 'Invalid file format. Please upload an image file.', | ||
'image_upload_component.error_file_size_exceeded': 'File size exceeds 10MB limit.', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked, and I have some JPEGs straight out of my camera that are larger than this. Is there any reason for this limit? Won't this image become way smaller after cropping and resolution-limiting anyway? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are correct, the cropped image that is saved becomes way smaller. But it seems like the browser doesn't handle it well when reading larger files, such as 50mbs. This happens when javascript tries to decode the full image into memory and allocate a texture in gpu memory when drawing the image in canvas that is used to display the cropping area. And during this both versions is stored temporarily.
After the load. the browser keeps struggling. So we ended up setting a limit on 10mb to ensure a good user experience on different browsers. With this context in mind, profile images, we thought that most images would probably would be below 10mb. But I tested a bit more, and it seem it maybe can handle up to 20mbs as well with a bit delay at the beginning. What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I see! I thought it might just be an arbitrary limit. It really comes down to your device then. Will it destroy performance even after cropping and uploading (or clearing the image), or is it permanent? If it's slow forever after I'm in favor of a limit, but otherwise maybe an alert is fine (something like the alert for animations)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like it works fine after I manage to save it, but before that everything lags enormously 😄 Panning the cropping area or using zoom can be extremely slow, and one time I got a browser error. I tested with 50mb from https://examplefile.com/image/jpg. As you mention, it could be an good option to display an alert message similar to animations if it exceeds 10mbs (something like: "Bildet er større enn 10 MB. Du kan oppleve at redigering/nettleseren går saktere enn vanlig."). I’m still a bit in favor of limiting the size, since the UX overall is pretty bad until the user manages to save, but I can sleep on it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No worries! I'll defer to your judgement here, sounds like you've been digging far deeper into this than me. 🙌 |
||
'image_upload_component.valid_file_types': 'Image files only', | ||
'input_components.remaining_characters': 'You have %d characters left', | ||
'input_components.exceeded_max_limit': 'You have exceeded the maximum limit with %d characters', | ||
'instance_selection.changed_by': 'Changed by', | ||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -70,7 +70,7 @@ export function nn() { | |||||||||
'form_filler.file_upload_valid_file_format_all': 'alle', | ||||||||||
'form_filler.file_uploader_add_attachment': 'Legg til fleire vedlegg', | ||||||||||
'form_filler.file_uploader_drag': 'Dra og slepp eller', | ||||||||||
'form_filler.file_uploader_find': 'leit etter fil', | ||||||||||
'form_filler.file_uploader_find': 'finn fil', | ||||||||||
'form_filler.file_uploader_list_delete': 'Slett vedlegg', | ||||||||||
'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', | ||||||||||
|
@@ -209,6 +209,17 @@ export function nn() { | |||||||||
'iframe_component.unsupported_browser_title': 'Nettlesaren din støttas ikkje', | ||||||||||
'iframe_component.unsupported_browser': | ||||||||||
'Nettlesaren di støttar ikkje iframes som brukar srcdoc. Dette kan føre til at du ikkje ser all innhaldet som er meint å visast her. Vi anbefalar deg å prøve ein annan nettlesar.', | ||||||||||
'image_upload_component.animated_warning': 'Hvis bildet er animert, vises bare det første bildet.', | ||||||||||
'image_upload_component.button_change': 'Bytt bilde', | ||||||||||
Comment on lines
+212
to
+213
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Translation inconsistencies: Bokmål used instead of Nynorsk. Lines 212 and 213 use Norwegian Bokmål rather than Nynorsk:
Apply this diff to correct the translations: - 'image_upload_component.animated_warning': 'Hvis bildet er animert, vises bare det første bildet.',
- 'image_upload_component.button_change': 'Bytt bilde',
+ 'image_upload_component.animated_warning': 'Viss bildet er animert, blir berre det første bildet vist.',
+ 'image_upload_component.button_change': 'Byt bilde', 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||
'image_upload_component.button_delete': 'Slett bildet', | ||||||||||
'image_upload_component.button_save': 'Lagre bilde', | ||||||||||
'image_upload_component.slider_zoom': 'Tilpass bildet', | ||||||||||
'image_upload_component.crop_area': 'Beskjæringsområde', | ||||||||||
'image_upload_component.summary_empty': 'Du har ikkje lasta opp noko bilde', | ||||||||||
'image_upload_component.reset': 'Tilbakestill zoom og plassering', | ||||||||||
'image_upload_component.error_invalid_file_type': 'Ugyldig filformat. Last opp ein bildefil.', | ||||||||||
'image_upload_component.error_file_size_exceeded': 'Fila er for stor. Største tillatte filstorleik er 10MB.', | ||||||||||
'image_upload_component.valid_file_types': 'Bildefiler er tillatne', | ||||||||||
'input_components.remaining_characters': 'Du har %d teikn igjen', | ||||||||||
'input_components.exceeded_max_limit': 'Du har overskride maks teikn med %d', | ||||||||||
'instance_selection.changed_by': 'Endra av', | ||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,3 +11,7 @@ video { | |
.cardMedia:last-of-type { | ||
margin-top: auto; | ||
} | ||
|
||
.mediaCard { | ||
margin-bottom: -7px; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
.canvas { | ||
display: block; | ||
position: relative; | ||
left: 50%; | ||
transform: translateX(-50%); | ||
cursor: grab; | ||
touch-action: none; | ||
} | ||
|
||
.canvas:active { | ||
cursor: grabbing; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
import React from 'react'; | ||
|
||
import { useLanguage } from 'src/features/language/useLanguage'; | ||
import { useCanvasDraw } from 'src/layout/ImageUpload/ImageCanvas/hooks/useCanvasDraw'; | ||
import { useDragInteraction } from 'src/layout/ImageUpload/ImageCanvas/hooks/useDragInteraction'; | ||
import { useKeyboardNavigation } from 'src/layout/ImageUpload/ImageCanvas/hooks/useKeyboardNavigation'; | ||
import { useZoomInteraction } from 'src/layout/ImageUpload/ImageCanvas/hooks/useZoomInteraction'; | ||
import classes from 'src/layout/ImageUpload/ImageCanvas/ImageCanvas.module.css'; | ||
import { ImagePreview } from 'src/layout/ImageUpload/ImageCanvas/ImagePreview'; | ||
import { useImageFile } from 'src/layout/ImageUpload/useImageFile'; | ||
import type { CropArea, Position } from 'src/layout/ImageUpload/imageUploadUtils'; | ||
interface ImageCanvasProps { | ||
imageRef: React.RefObject<HTMLImageElement | null>; | ||
zoom: number; | ||
position: Position; | ||
cropArea: CropArea; | ||
baseComponentId: string; | ||
onPositionChange: (newPosition: Position) => void; | ||
onZoomChange: (newZoom: number) => void; | ||
canvasRef: React.RefObject<HTMLCanvasElement | null>; | ||
} | ||
|
||
const CANVAS_HEIGHT = 320; | ||
const CANVAS_WIDTH = 1000; | ||
|
||
export function ImageCanvas({ | ||
imageRef, | ||
zoom, | ||
position, | ||
cropArea, | ||
baseComponentId, | ||
onPositionChange, | ||
onZoomChange, | ||
canvasRef, | ||
}: ImageCanvasProps) { | ||
const { storedImage, imageUrl } = useImageFile(baseComponentId); | ||
const { langAsString } = useLanguage(); | ||
|
||
useCanvasDraw({ canvasRef, imageRef, zoom, position, cropArea }); | ||
useZoomInteraction({ canvasRef, zoom, onZoomChange }); | ||
const { handlePointerDown } = useDragInteraction({ canvasRef, position, onPositionChange }); | ||
const { handleKeyDown } = useKeyboardNavigation({ position, onPositionChange }); | ||
|
||
if (storedImage) { | ||
return ( | ||
<ImagePreview | ||
storedImage={storedImage} | ||
imageUrl={imageUrl} | ||
/> | ||
); | ||
} | ||
|
||
return ( | ||
<canvas | ||
onPointerDown={handlePointerDown} | ||
onKeyDown={handleKeyDown} | ||
tabIndex={0} | ||
ref={canvasRef} | ||
height={CANVAS_HEIGHT} | ||
width={CANVAS_WIDTH} | ||
className={classes.canvas} | ||
aria-label={langAsString('image_upload_component.crop_area')} | ||
/> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
.previewWrapper { | ||
background-color: #f4f5f6; /* Following does not exist in v1: var(--ds-color-neutral-background-subtle); */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we use a different color here? If this color already exists elsewhere, we should align with that and reference a shared CSS variable/design token. If it’s specific to this component only, consider introducing a v1 component-scoped token instead of hard-coding the value. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We will look for a similar token instead :) |
||
height: 320px; | ||
display: flex; | ||
justify-content: center; | ||
align-items: center; | ||
padding: var(--ds-size-4); | ||
box-sizing: border-box; | ||
} |
Uh oh!
There was an error while loading. Please reload this page.