Add script to convert images from images to out folder by Genne23v · Pull Request #30 · Seneca-CDOT/my-photohub

Genne23v · 2022-12-03T01:15:58Z

This PR is addressing #3.
I'm adding image-optimizer.js with this PR. But I think it will need much more discussion from here as I have some questions about the next step.

This script will get all files in root images folder and create resized images in out folder.
Resized mages will be 525 x 295, 746 x 420, 934 x 525. Please let me know if this is suitable. Probably I have to create more square-like images for image grid layout.
By resizing images with sharp library, it will clear most of metadata, but let me know if this is not the intention.
This script can be run with node image-optimizer.js.
To add an option to choose image type, I would like to further understand how it should be done. I'm assuming this app is deployed whenever the user sends a pull request. Please let me know if this assumption is right.

SerpentBytes · 2022-12-03T22:32:45Z

... I'm assuming this app is deployed whenever the user sends a pull request. Please let me know if this assumption is right.

As I understood the flow of the events, it's as follows:

User enters a valid GitHub PAT token and/or repo name
A repo is generated using the user-provided name (If no name is provided, we generate a name)
User selects photos they want to upload to the repo
The images are optimized using a script
Images are committed to the repo generated on behalf of the user previously.

Genne23v · 2022-12-03T23:16:43Z

Thanks for the comment @SerpentBytes. I'm still not sure how it is deployed to GitHub Pages, but let me start writing scripts.

Genne23v · 2022-12-04T01:24:17Z

@SerpentBytes Just to make sure I'm on the right track. My understanding is that the user go to my-photohub webpage and add credentials and photos. When upload button is pressed, the user's own repo is created and deployed to GitHub Page. Am I in the right direction?

SerpentBytes · 2022-12-04T02:10:46Z

@SerpentBytes Just to make sure I'm on the right track. My understanding is that the user go to my-photohub webpage and add credentials and photos. When upload button is pressed, the user's own repo is created and deployed to GitHub Page. Am I in the right direction?

To access the service, the user needs to go through the authentication which we are doing using a form which accepts PAT token and repo name. PAT token is required, but the repo name is not always. The repo name is required in a use case where a repo name is taken on the user's GitHub account. If that's the case, then the repo name is mandatory (with the current implementation.)

SerpentBytes · 2022-12-04T02:26:55Z

@Genne23v At the moment, only authentication and repo generation functionality is implemented. We have a UI for uploading photos, but it's not connected to the service.

humphd · 2022-12-05T20:34:16Z

We have a few options we can take here:

We could do this optimization in the browser before we upload them to the repo. This could be done with something like https://www.npmjs.com/package/browser-image-compression or https://github.com/fengyuanchen/compressorjs or using some WASM-based library (this is what https://squoosh.app/ does via its API).
We could continue with your approach, and do this server-side. The down side of this is that the images have already been sent, along with their metadata.

Regardless of which way we go, I think you should consider the file types and sizes to generate. You're picking a few sizes, but they are quite small, and they include both a width and height. This will skew images that don't have the same aspect ratio as your final dimensions. We should probably use the <picture> element, and provide <source> elements to include various sizes the browser can pick from at runtime.

I also think we need more than WebP. Probably we should do: JPEG, WebP, AVIF, PNG.

RE: your code, you are mixing async, sync fs methods. I would use async everywhere or promises.

Genne23v · 2022-12-05T20:41:27Z

Thank you for the feedback. I'm combining UI and GitHub repo generation script along with image optimizer. So I will replace server-side image generation work with your suggestion. Basically my change will do

Hit Upload button
Validate PAT and repo name and show appropriate error message on UI
If credential and repo name are valid, create optimized images
Add and commit to the repo

Please let me have any advise if you have. Thank you!

humphd · 2022-12-05T20:42:42Z

I think this sounds right, yes.

Our UI will have a bunch of work to do, so we'll need "progress" indication to be part of it, so people don't close the window.

Genne23v · 2022-12-10T23:47:41Z

@humphd @SerpentBytes
I have added some updates as below.

By hitting Upload button, it will start image optimization, creating a repo and making a commit
I utilized some of the assets in worker folder. I copied the worker folder inside the src
This will create png, webp, jpg, avif with the sizes of 375, 744, 950, 1120
Added userMessage so that user can be aware what's going

Please try the feature and let me know if there is any improvement that I need to make. Thanks

Genne23v · 2022-12-10T23:48:32Z

Also I noticed there is a merge conflict. It seems like I don't have an authority to fix this. Please let me know if I need to do something.

Genne23v · 2022-12-11T18:32:07Z

@humphd I have updated DataURL to BlobURL. Please let me know if there is any further improvement needed. Thank you

Genne23v · 2022-12-11T18:51:50Z

@SerpentBytes As I mentioned above, I copied worker folder with a little modification inside src. Will it be an issue if I remove worker folder in the root?

SerpentBytes · 2022-12-11T19:01:26Z

@Genne23v Make sure to update the package.json scripts associated with the worker, i.e.: auth-worker and auth-worker:login. Also, since eslint configuration is different for the contents of the worker folder, could you please ensure that we don't run into linting errors? Thank you.

Genne23v · 2022-12-11T19:51:24Z

I added your incremental lint rule to the .eslintrc.js in the root along with npm run auth-worker script. And I checked auth-worker scripts work fine. Thank you

humphd

This is awesome, nice work! A few comments.

humphd · 2022-12-15T16:02:18Z

src/UploadInterface.js

    <div className="App">
      <header className="preview">
+        {!userMessage ? (
+          <></>


null is more common when you want to show nothing.

Will be addressed

humphd · 2022-12-15T16:03:35Z

src/UploadInterface.js

            };
-            reader.readAsDataURL(file);
-            imgPreviewEl.current.title = file.name;
+            reader.readAsDataURL(files[0]);


Question: on line 137 you allow selecting multiple files, but here assume it's only 1 (i.e., files[0]). Should you be looping through them?

If I understand this code correctly, it should read only one image and add it to imgPreviewEl.current.src to load the image on the element. If I change it to files, it doesn't load the preview on the element. I think it doesn't need to read every file unless if we want to show multiple previews.

humphd · 2022-12-15T16:08:23Z

src/worker/functions/commit-files.js

+      reader.onload = async () => {
+        const imageBuffer = reader.result;
+        const byteArray = new Uint8Array(imageBuffer);
+        const blob = new Blob([byteArray]);


Since you know the file types we allow (i.e., image/*), why don't we pass the mimeType here too as second arg: https://developer.mozilla.org/en-US/docs/Web/API/Blob/Blob

Will be addressed accordingly

humphd · 2022-12-15T16:08:35Z

src/worker/functions/commit-files.js

+        const imageBuffer = reader.result;
+        const byteArray = new Uint8Array(imageBuffer);
+        const blob = new Blob([byteArray]);
+        const BlobURL = URL.createObjectURL(blob);


lowerCamelCase for naming in JS

Will be handled

humphd · 2022-12-15T16:10:26Z

src/worker/functions/commit-files.js

+        const byteArray = new Uint8Array(imageBuffer);
+        const blob = new Blob([byteArray]);
+        const BlobURL = URL.createObjectURL(blob);
+        const blobData = octokit.request(`POST /repos/${username}/${repoName}/git/blobs`, {


Either you need to await this call, or better, you should return it as the Promise to wait on in line 48 below.

Will be addressed

humphd · 2022-12-15T16:11:48Z

This needs a rebase too.

Genne23v · 2022-12-15T19:36:18Z

This needs a rebase too.

I rebased my branch and it shows one commit in my view. Also I will make sure there is one commit in next one. Please let me know something is still not right. Thanks

Genne23v · 2022-12-15T22:09:12Z

@humphd I have added camelCase dependency to transform the file name into lowerCamelCase. Image width is followed without underscore. Please let me know if this one is not the standard.

SerpentBytes assigned Genne23v Dec 4, 2022

SerpentBytes added the ✨ enhancement New feature or request label Dec 4, 2022

SerpentBytes requested a review from humphd December 11, 2022 02:11

SerpentBytes requested review from CDPhu, batunpc and saminarp December 11, 2022 19:54

Genne23v force-pushed the issue-3 branch from a19f1e5 to 004e0fa Compare December 12, 2022 17:32

humphd requested changes Dec 15, 2022

View reviewed changes

Genne23v force-pushed the issue-3 branch from 004e0fa to 8d9299a Compare December 15, 2022 22:05

Add script converting image and committing to repo

0f7b857

Genne23v force-pushed the issue-3 branch from 8d9299a to 0f7b857 Compare December 15, 2022 22:08

Conversation

Genne23v commented Dec 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SerpentBytes commented Dec 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Genne23v commented Dec 3, 2022

Uh oh!

Genne23v commented Dec 4, 2022

Uh oh!

SerpentBytes commented Dec 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SerpentBytes commented Dec 4, 2022

Uh oh!

humphd commented Dec 5, 2022

Uh oh!

Genne23v commented Dec 5, 2022

Uh oh!

humphd commented Dec 5, 2022

Uh oh!

Genne23v commented Dec 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Genne23v commented Dec 10, 2022

Uh oh!

Genne23v commented Dec 11, 2022

Uh oh!

Genne23v commented Dec 11, 2022

Uh oh!

SerpentBytes commented Dec 11, 2022

Uh oh!

Genne23v commented Dec 11, 2022

Uh oh!

humphd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

humphd commented Dec 15, 2022

Uh oh!

Genne23v commented Dec 15, 2022

Uh oh!

Genne23v commented Dec 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Genne23v commented Dec 3, 2022 •

edited

Loading

SerpentBytes commented Dec 3, 2022 •

edited

Loading

SerpentBytes commented Dec 4, 2022 •

edited

Loading

Genne23v commented Dec 10, 2022 •

edited

Loading