Improve React example #15

igoradamenko · 2023-10-25T15:25:56Z

Hey there!

@tenebricosa and I have rewritten React example a bit. The main point is to make the example look like a part of a real world React app.

We're ready to your review :-)

Improve React example

igoradamenko · 2023-10-25T15:26:40Z

@nd0ut, I do not have enough rights to add you as a reviewer 🐨

examples/react-uploader/src/App.module.scss

igoradamenko · 2023-10-25T15:30:15Z

examples/react-uploader/src/mocks.ts

+...`,
+  photos: [
+    {
+      uuid: 'e456f8cd-a717-4894-932d-e1d39e043e50',


I will also reupload the images. Currently they are uploaded to my own account, if I recall correctly.

igoradamenko · 2023-10-26T10:07:12Z

examples/react-uploader/package.json

    "@vitejs/plugin-react": "^3.1.0",
+    "prop-types": "15.8.1",


For some reason, when I install deps of this project via npm, everything works fine.

But when I use yarn, Vite screams that it can not find prop-types for react-toggle.

So I decided simply install the dep to make sure that users won't get the same error.

react-toggle has prop-types listed in its peer dependencies, and yarn handles them more strictly

nd0ut · 2023-10-27T14:21:36Z

examples/react-uploader/src/types.ts

@@ -0,0 +1,6 @@
+export type File = {


In the latest 0.29.0 version, there is an OutputFileEntry type exported that could be used there.

Now when I'm trying to use the data I have got from LR_DATA_OUTPUT, TS screams that types are not compatible:

src/App.tsx:27:60 - error TS2345: Argument of type '{ uuid: string; name: string; size: number; isStored: boolean; isImage: boolean; mimeType: string; cdnUrl: string; s3Url: null; origin alFilename: string; imageInfo: { dpi: number[]; width: number; format: string; ... 5 more ...; datetimeOriginal: null; }; ... 13 more ...; uploadProgress: number; }[]' is not assignable to parameter of type 'OutputFileEntry[] | (() => OutputFileEntry[])'. Type '{ uuid: string; name: string; size: number; isStored: boolean; isImage: boolean; mimeType: string; cdnUrl: string; s3Url: null; originalFilename: string; imageInfo: { dpi: number []; width: number; format: string; ... 5 more ...; datetimeOriginal: null; }; ... 13 more ...; uploadProgress: number; }[]' is not assignable to type 'OutputFileEntry[]'. Type '{ uuid: string; name: string; size: number; isStored: boolean; isImage: boolean; mimeType: string; cdnUrl: string; s3Url: null; originalFilename: string; imageInfo: { dpi: numb er[]; width: number; format: string; ... 5 more ...; datetimeOriginal: null; }; ... 13 more ...; uploadProgress: number; }' is not assignable to type 'OutputFileEntry'. Type '{ uuid: string; name: string; size: number; isStored: boolean; isImage: boolean; mimeType: string; cdnUrl: string; s3Url: null; originalFilename: string; imageInfo: { dpi: nu mber[]; width: number; format: string; ... 5 more ...; datetimeOriginal: null; }; ... 13 more ...; uploadProgress: number; }' is not assignable to type 'Partial<Omit<UploadcareFile, requ iredFileInfoFields>>'. The types of 'imageInfo.datetimeOriginal' are incompatible between these types. Type 'null' is not assignable to type 'string'. 27 const [photos, setPhotos] = useState<FormType['photos']>(MOCK_DATA.photos);

imageInfo.datetimeOriginal returned from the File Uploader is null. But the types say it must be string.

Pushed the changes anyway. See a6a08d6.

Now when I'm trying to use the data I have got from LR_DATA_OUTPUT, TS screams that types are not compatible:

Got it. This is a bug inside upload-client. I'll fix it soon.

Link uploadcare/uploadcare-js-api-clients#489

Link uploadcare/file-uploader#550

nd0ut · 2023-10-30T09:25:21Z

examples/react-uploader/src/FileUploader/FileUploader.tsx

+            <img
+              className={st.previewImage}
+              key={file.uuid}
+              src={`https://ucarecdn.com/${file.uuid}/-/preview/-/resize/x200/`}


Let's use cdnUrlModifiers here to build preview url (or cdnUrl)

When I upload a file, I get cdnUrlModifiers as null.

So I used cdnUrl.

Hmm, looks like a bug. I'll check it.

As far as I understand, it returns null when the uploaded image was not changed via editor. Which complies with types:

cdnUrlModifiers: string | null;

It may be fine, but due to null I will have to check it prior to using in the URL template. So, I decided to use cdnUrl instead.

nd0ut · 2023-10-30T10:38:59Z

examples/react-uploader/src/mocks.ts

+...`,
+  photos: [
+    {
+      uuid: 'add046ce-95e3-4454-bccd-65e3e648c472',


Why do you store all this stuff here? Isn't just uuid/url/name enough?

This is the list of mocks. I believe mocks should be designed to match the real data as much as they can.

Plus it provides data consistency. I use these mocks as an initial value for the state. Later I add data returned by File Uploader to the same state. It would be weird to have differently structured objects inside one state array.

Well, I could probably use only subset of the fields here, and extract the same subset from the objects of the array returned by File Uploader. I do no think this is a good idea though, due to two reasons:

This will increase the number of code lines of the logic. I'd like to leave as less as possible of them, to make the example more vivid / dry.

I believe that the real life example would be closer to “let's store everything this thing returns” rather than storing UUID only. Because you never know what data you will need tomorrow, and it's always easy to remove excessive chunks from your DB.

nd0ut · 2023-10-30T10:40:06Z

examples/react-uploader/src/FileUploader/FileUploader.tsx

+
+
+  return (
+    <div className={st.root}>


It's better to use uploadCollection.clearAll() on lr-upload-ctx-provider

nd0ut · 2023-10-30T10:55:20Z

examples/react-uploader/src/FileUploader/FileUploader.tsx

+    const handleUploadEvent = (e: CustomEvent<{ data: OutputFileEntry[] }>) => {
+      if (e.detail?.data) {
+        const newUploadedFiles = e.detail.data.filter(file => file.isUploaded && !files.find(f => f.uuid === file.uuid));
+        onChange([...files, ...newUploadedFiles]);


Why do you call the onChange within the LR_DATA_OUTPUT event? Wouldn't it be more appropriate to do this within LR_DONE_FLOW? As the 'done' click represents a sort of "commit transaction" action in the regular mode. When a modal is shown, there seems to be no need to update any outer UI. Perhaps it might be better to switch to the inline or minimal mode instead?

Why do you call the onChange within the LR_DATA_OUTPUT event?

My initial idea was to use LR_DATA_OUTPUT, because it's easier this way.

LR_DONE_FLOW returns nothing:

window.addEventListener('LR_DONE_FLOW', e => console.log(e));

{ "ctx": "my-uploader", "data": undefined, "type": "DONE_FLOW" }

The documentation says nothing about the ways to force LR_DONE_FLOW to return anything.

As far as I understand, it's intentional and not the lack of docs: https://github.com/uploadcare/blocks/blob/141f1edfdd6ccec7577b142927dfb219e8962d2a/abstract/UploaderBlock.js#L289-L296

If I triggered the onChange on LR_DONE_FLOW, then I would need to store the internal state of the uploader somehow. And I decided to get rid of this for the sake of brevity.

However, now I understand that this code is buggy. It filters files using UUID, which won't be changed when user applies transformations to an image. So, I've updated it: a108a84

Yep, LR_DONE_FLOW is intended to be just event without payload. But I'm sure, it's way more useful if it will provide files array as LR_DATA_OUTPUT do.

nd0ut · 2023-10-30T14:13:05Z

Great job! 🔥 Let's wait for this PR, update blocks to ensure the types are OK, and then we're good to go!

igoradamenko · 2023-10-31T11:55:04Z

Let's wait for this uploadcare/file-uploader#550, update blocks to ensure the types are OK, and then we're good to go!

@nd0ut, I've updated Blocks to v0.29.1 (caa5150). But now types are incompatible in a different place.

src/App.tsx:27:60 - error TS2345: Argument of type '{ uuid: string; name: string; size: number; isStored: boolean; isImage: boolean; mimeType: string; cdnUrl: string; s3Url: null; origin
alFilename: string; imageInfo: { dpi: number[]; width: number; format: string; ... 5 more ...; datetimeOriginal: null; }; ... 4 more ...; cdnUrlModifiers: null; }[]' is not assignable to
 parameter of type 'OutputFileEntry[] | (() => OutputFileEntry[])'.
  Type '{ uuid: string; name: string; size: number; isStored: boolean; isImage: boolean; mimeType: string; cdnUrl: string; s3Url: null; originalFilename: string; imageInfo: { dpi: number
[]; width: number; format: string; ... 5 more ...; datetimeOriginal: null; }; ... 4 more ...; cdnUrlModifiers: null; }[]' is not assignable to type 'OutputFileEntry[]'.
    Type '{ uuid: string; name: string; size: number; isStored: boolean; isImage: boolean; mimeType: string; cdnUrl: string; s3Url: null; originalFilename: string; imageInfo: { dpi: numb
er[]; width: number; format: string; ... 5 more ...; datetimeOriginal: null; }; ... 4 more ...; cdnUrlModifiers: null; }' is not assignable to type 'OutputFileEntry'.
      Type '{ uuid: string; name: string; size: number; isStored: boolean; isImage: boolean; mimeType: string; cdnUrl: string; s3Url: null; originalFilename: string; imageInfo: { dpi: nu
mber[]; width: number; format: string; ... 5 more ...; datetimeOriginal: null; }; ... 4 more ...; cdnUrlModifiers: null; }' is not assignable to type 'Partial<Omit<UploadcareFile, requir
edFileInfoFields>>'.
        The types of 'imageInfo.dpi' are incompatible between these types.
          Type 'number[]' is missing the following properties from type '{ "0": number; "1": number; }': "0", "1"

27   const [photos, setPhotos] = useState<FormType['photos']>(MOCK_DATA.photos);

The log tries to say that this:

https://github.com/uploadcare/blocks-examples/blob/caa5150c6cae3b7a0dd8142f95e5e0ffb5e8c1fa/examples/react-uploader/src/mocks.ts#L20-L23

does not match this:

https://github.com/uploadcare/uploadcare-js-api-clients/blob/8e06a32ae111dd6698b8a370dd56e6f4a1076a94/packages/api-client-utils/src/types/ImageInfo.ts#L13-L16

I believe it happens because types are trying to limit dpi number array length. I can not mark mocks object as const, because this way I will get:

src/App.tsx:27:60 - error TS2345: Argument of type 'readonly [{ readonly uuid: "add046ce-95e3-4454-bccd-65e3e648c472"; readonly name: "arthurhumeau3xwdarHxHqIunsplash.jpg"; readonly size
: 1657550; readonly isStored: true; readonly isImage: true; ... 9 more ...; readonly cdnUrlModifiers: null; }, ... 4 more ..., { ...; }]' is not assignable to parameter of type 'OutputFi
leEntry[] | (() => OutputFileEntry[])'.
  The type 'readonly [{ readonly uuid: "add046ce-95e3-4454-bccd-65e3e648c472"; readonly name: "arthurhumeau3xwdarHxHqIunsplash.jpg"; readonly size: 1657550; readonly isStored: true; read
only isImage: true; ... 9 more ...; readonly cdnUrlModifiers: null; }, ... 4 more ..., { ...; }]' is 'readonly' and cannot be assigned to the mutable type 'OutputFileEntry[]'.

27   const [photos, setPhotos] = useState<FormType['photos']>(MOCK_DATA.photos);

Sooo, I've extracted [72, 72] array to a variable and marked it as const. Then I've replaced all these arrays with the variable in the mocks.

Not sure it's a right way though. But anyway: 34265d9

nd0ut · 2023-11-01T10:23:12Z

Not sure it's a right way though. But anyway: 34265d9

It's possible to define dpi like this:

{
  dpi: [72, 72] as NonNullable<OutputFileEntry["imageInfo"]>["dpi"];
}

or even better like this:

{
  photos: [
    {
      uuid: "add046ce-95e3-4454-bccd-65e3e648c472",
      name: "arthurhumeau3xwdarHxHqIunsplash.jpg",
      size: 1657550,
      isStored: true,
      isImage: true,
      mimeType: "image/jpeg",
      cdnUrl: "https://ucarecdn.com/add046ce-95e3-4454-bccd-65e3e648c472/",
      s3Url: null,
      originalFilename: "arthur-humeau-3xwdarHxHqI-unsplash.jpg",
      imageInfo: {
        dpi: [72, 72],
        width: 2400,
        format: "JPEG",
        height: 3600,
        sequence: false,
        colorMode: "RGB",
        orientation: null,
        geoLocation: null,
        datetimeOriginal: null,
      },
      videoInfo: null,
      contentInfo: {
        mime: {
          mime: "image/jpeg",
          type: "image",
          subtype: "jpeg",
        },
        image: {
          dpi: [72, 72],
          width: 2400,
          format: "JPEG",
          height: 3600,
          sequence: false,
          colorMode: "RGB",
          orientation: null,
          geoLocation: null,
          datetimeOriginal: null,
        },
      },
      metadata: {},
      s3Bucket: null,
      cdnUrlModifiers: null,
      // those properties are missing in your mocks so types are incompatible
      validationErrorMessage: null,
      file: null,
      uploadError: null,
      uploadProgress: 100,
      isValid: false,
      isUploaded: false,
      fullPath: null,
      externalUrl: null,
    },
  ] as OutputFileEntry[];
}

igoradamenko · 2023-11-01T11:01:53Z

@nd0ut, I typed the whole mock object. Now it looks cleaner, thanks 🐨

4aa2efe

igoradamenko and others added 17 commits October 23, 2023 17:23

Change HTML title in the example

748ef28

Update Blocks and add classnames to deps

bebbd69

Restyle the page

aac2340

Add form mocks

1a53893

Create FileUploader component

e799ea3

Restyle App and use FileUploader there

60f49a0

Add react-toggle and style it

547da36

Restyle the app and the file uploader depending on theme switcher

8439d28

Update texts to make them photo-related

95e066f

Add important notes and README

b6fb8af

Reset FileUploader state on every use

686cae9

Use imported CSS instead of CDN URL

37844ec

Resize theme switch images

85a7021

Make theme switch adaptive to theme too

385ac64

Update README

4686356

Fix typos

72a52e6

Merge pull request #1 from igoradamenko/improvement/react-example

b0b79ca

Improve React example

igoradamenko commented Oct 25, 2023

View reviewed changes

examples/react-uploader/src/App.module.scss Outdated Show resolved Hide resolved

igoradamenko commented Oct 25, 2023

View reviewed changes

Complete "localization"

6f8d3c0

nd0ut self-requested a review October 26, 2023 09:42

igoradamenko added 3 commits October 26, 2023 12:49

Reupload images to Uploadcare account

ba5d133

Install prop-types for react-toggle

ee9edc3

Add local running instructions

1a70845

igoradamenko commented Oct 26, 2023

View reviewed changes

Add sass to react-uploader deps

6b503b1

nd0ut reviewed Oct 27, 2023

View reviewed changes

igoradamenko added 2 commits October 29, 2023 17:46

Update Blocks to v0.29.0

a6a08d6

Remove file limits

714c409

igoradamenko requested a review from nd0ut October 29, 2023 15:48

nd0ut reviewed Oct 30, 2023

View reviewed changes

Use cdnUrl field instead of inlining CDN domain

0341489

nd0ut reviewed Oct 30, 2023

View reviewed changes

igoradamenko added 2 commits October 30, 2023 12:55

Use lr-upload-ctx-provider methods to clear File Uploader state

81fced6

Trigger onChange on LR_DONE_FLOW instead of LR_DATA_OUTPUT

a108a84

nd0ut self-requested a review October 30, 2023 14:13

nd0ut approved these changes Oct 30, 2023

View reviewed changes

Update Blocks to v0.29.1

caa5150

Update mocks to fix types

4aa2efe

nd0ut merged commit 22e71e7 into uploadcare:main Nov 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve React example #15

Improve React example #15

igoradamenko commented Oct 25, 2023

igoradamenko commented Oct 25, 2023

igoradamenko Oct 25, 2023

igoradamenko Oct 26, 2023

nd0ut Oct 30, 2023

nd0ut Oct 27, 2023

igoradamenko Oct 29, 2023

igoradamenko Oct 29, 2023

nd0ut Oct 30, 2023

nd0ut Oct 30, 2023

nd0ut Oct 30, 2023

nd0ut Oct 30, 2023

igoradamenko Oct 30, 2023

nd0ut Oct 30, 2023

igoradamenko Oct 30, 2023

nd0ut Oct 30, 2023

igoradamenko Oct 30, 2023

nd0ut Oct 30, 2023

igoradamenko Oct 30, 2023

nd0ut Oct 30, 2023 •

edited

Loading

igoradamenko Oct 30, 2023 •

edited

Loading

nd0ut Oct 30, 2023

nd0ut commented Oct 30, 2023

igoradamenko commented Oct 31, 2023

nd0ut commented Nov 1, 2023

igoradamenko commented Nov 1, 2023

Improve React example #15

Improve React example #15

Conversation

igoradamenko commented Oct 25, 2023

igoradamenko commented Oct 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nd0ut Oct 30, 2023 • edited Loading

Choose a reason for hiding this comment

igoradamenko Oct 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nd0ut commented Oct 30, 2023

igoradamenko commented Oct 31, 2023

nd0ut commented Nov 1, 2023

igoradamenko commented Nov 1, 2023

nd0ut Oct 30, 2023 •

edited

Loading

igoradamenko Oct 30, 2023 •

edited

Loading