Allow user to upload reference images #60
Conversation
There was a problem hiding this comment.
Nice work! The one issue I've been having is with PDF files. When I select them, they do show up in the list on the sidebar, but not on the map. Are you seeing this on your end as well? Or do I just have bad PDFs?
I would recommend investigating that for a bit, but if it ends up being too complex we should defer it to a follow up card.
| const imageShouldBeAdded = url => | ||
| !(url in referenceImageLayers.current); | ||
|
|
||
| const imageShouldBeHidden = url => !(url in visibleImages); |
|
|
||
| const imageShouldBeHidden = url => !(url in visibleImages); | ||
|
|
||
| for (const [url, { corners, mode }] of Object.entries(visibleImages)) { |
| state.referenceImage = { ...state.referenceImage, ...update }; | ||
| updateReferenceImage: (state, { payload: { url, update } }) => { | ||
| state.referenceImages[url] = { | ||
| ...(state.referenceImages?.[url] ?? {}), |
There was a problem hiding this comment.
Nice use of optional chaining and null coalescing
I forgot to test this. I just tried now and it's not working for me either. I don't think DistortableImage supports using PDFs. I think we have a couple of options here: It seems like it should be possible to convert the PDF to an image with Mozilla's PDF.js. I checked the npmjs page for the distributable, pdfjs-dist, and the unpacked size is 34.7 MB (yes, MB). The package is available from a CDN, but users would still need to download the full library. The other option would be to add a route to the django server to convert pdfs and send back images. This adds complexity to the pdf upload process, but also doesn't require the user to download any additional libraries. |
|
Nice investigation. I made #62 for adding PDF support in a follow up card. We can prioritize it later. Going to give this a final look. |
|
For the long file names that are cutoff, let's add a native tooltip on hover which shows the full file name for now. |
There was a problem hiding this comment.
+1 tested, this looks really good. Even tried uploading the same file twice, and the sidebar, despite having two same named files, did not get confused between them.
Let's add a tooltip for the long file names here, and then this will be ready to go.
Please hold off on merging until after the client demo ends at 1pm.
|
Taking another look |
|
ba7215e removes |
This commit updates the ReferenceLayer component to store the individual reference image layers in a mutable react ref. This cuts down on the number of times these layers are re-created; a high number of re-creations has caused unpredictable behavior. I first tried using L.DistortableImage's distortableCollection, but this would make the reference images uneditable.
mstone121
force-pushed
the
ms/implement-user-image-upload
branch
from
ba7215e
to
e44208d
Compare
Overview
This PR adds the ability for the user to upload their own images. These images will appear as toggleable reference layers in the sidebar.
Closes #54
Demo
Screen.Recording.2022-09-07.at.5.00.04.PM.mov
Notes
This PR updates the ReferenceLayer component to store the individual reference image layers in a mutable react ref. This cuts down on the number of times these layers are re-created; a high number of re-creations has caused unpredictable behavior.
I first tried using L.DistortableImage's distortableCollection to store multiple distortable layers, but this would make the reference images uneditable.
As is evident in the demo video above, if the file names are long, they are cutoff by the end of the sidebar. This should be addressed especially as reference images may come in sets with files that begin with the same prefix (e.g. see demo).
Testing Instructions
Checklist
fixup!commits have been squashedCHANGELOG.mdupdated with summary of features or fixes, following Keep a Changelog guidelinesREADME.mdupdated if necessary to reflect the changes