Skip to content
This repository has been archived by the owner on May 12, 2023. It is now read-only.

feat: image transform support #46

Merged
merged 34 commits into from Dec 13, 2022
Merged

feat: image transform support #46

merged 34 commits into from Dec 13, 2022

Conversation

dshukertjr
Copy link
Member

@dshukertjr dshukertjr commented Dec 2, 2022

What kind of change does this PR introduce?

New image transformation feature will be added to Supabase. This PR adds support for that feature.

  • transform option has been added to getPublicUrl and createSignedUrl methods.
  • download method has been deprecated in favor of publicDownload and authenticatedDownload methods.

supabase-js code here
supabase/storage-js#129

What is the current behavior?

Image transform is not supported

What is the new behavior?

Image transform feature is supported where images stored on Supabase storage can be resized on the fly.

We will hold off on merging this until this PR is merged on storage-js.

@dshukertjr dshukertjr marked this pull request as ready for review December 6, 2022 06:24
lib/src/storage_file_api.dart Outdated Show resolved Hide resolved
lib/src/types.dart Outdated Show resolved Hide resolved
@@ -205,3 +205,46 @@ class StorageRetryController {
_cancelled = true;
}
}

enum TransformCrop { fill, fit, fillDown, force, auto }
Copy link
Contributor

Choose a reason for hiding this comment

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

What about some documentation about these options? Or should they only be documented on the supabase docs website?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, you're right. Let me add more documentations around these types and more.

Copy link
Member Author

@dshukertjr dshukertjr Dec 6, 2022

Choose a reason for hiding this comment

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

@Vinzent03 I also wonder if TransformCrop is a good name... Which one do you think is the best?

  • TransformCrop
  • CropOption
  • CropType
  • CropMethod
    or other?

Copy link
Member Author

Choose a reason for hiding this comment

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

I personally kind of like CropOption now that I see a few options like this.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems like mode is the term used across industries. I think I will go with ResizeMode here after, but what do you think?

https://docs.imgix.com/apis/rendering/size/fit
https://cloudinary.com/documentation/resizing_and_cropping#resize_and_crop_modes

Copy link
Member Author

Choose a reason for hiding this comment

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

Just chatted with the storage team and confirmed that cover, contain, and fill are the finalized resize modes.

Copy link
Contributor

@Vinzent03 Vinzent03 Dec 7, 2022

Choose a reason for hiding this comment

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

Okay great. I really like the images in the documentation of BoxFit in Flutter. Do you think we can do the same for the resize modes? Or do you think that's overkill.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dshukertjr In case you missed the above message.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Vinzent03 Thanks for pinging me! Yeah, that is a great idea! Let me add something similar!

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, it might be a bit time consuming, so let's come back to that nice idea after launch week!

lib/src/types.dart Outdated Show resolved Hide resolved
Co-authored-by: Bruno D'Luka <45696119+bdlukaa@users.noreply.github.com>
@dshukertjr
Copy link
Member Author

dshukertjr commented Dec 11, 2022

Just an FYI, there will most likely be some last minute API change on the storage-js side. Specifically, we might not deprecate download() method, and just keep on using it instead of introducing publicDownload() and authenticatedDownload(). I will reflect those changes once they are confirmed and merged into storage-js library!

lib/src/storage_file_api.dart Outdated Show resolved Hide resolved
Co-authored-by: Vinzent <vinzent03@proton.me>
@dshukertjr
Copy link
Member Author

We will release a new version of storage-dart tomorrow. Will update the change logs and stuff so that we can release this version right after merging this PR!

@dshukertjr dshukertjr merged commit d9ecad8 into main Dec 13, 2022
@dshukertjr dshukertjr deleted the feat/transform branch December 13, 2022 06:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants