Skip to content
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

Add image field type #47

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

roidayan
Copy link
Contributor

Works the same as file field type but with a preview of the image.

Signed-off-by: Roi Dayan [email protected]

@roidayan roidayan force-pushed the add_image_field_type branch from f76a5a3 to 20b9b57 Compare May 20, 2015 09:25
@roidayan roidayan mentioned this pull request May 21, 2015
@roidayan roidayan force-pushed the add_image_field_type branch from 20b9b57 to 47781d7 Compare May 24, 2015 07:35
@roidayan
Copy link
Contributor Author

hi, any comments?

@roidayan
Copy link
Contributor Author

roidayan commented Sep 8, 2016

Hi @tareq1988 any comments?

@tareq1988
Copy link
Owner

Sorry for the long delay on reviewing this. Ideally I would like to store the attachment ID instead of image URL as we might need various sizes of that image and finding an attachment ID with URL will be a nightmare. So we could pass the attachment ID as a hidden field, a thumbnail version of the image preview (right now it shows the full size). Also limit the media manager to image-only selection, we certainly don't want to select a zip file as an image file.

@roidayan
Copy link
Contributor Author

ok thanks for the comments.

@roidayan
Copy link
Contributor Author

The point at first was to be able to supply external url as well. With your suggestions only the button and preview are needed (without the input box). the media manager has an upload tab to add external file into the media manager.

@roidayan
Copy link
Contributor Author

another note. showing the original image was the point of showing the user the real preview of the size that is going to be used. i.e. i use it for adding image for email header and wanted the user to see preview of the image in real size. I guess this could be configurable of what kind of preview to show.

@tareq1988
Copy link
Owner

Consider a raw image uploaded, or a fairly medium sized 1200x800, it would overflow the whole settings area.

@roidayan
Copy link
Contributor Author

right, but in my example if i requested header email image then it can be considered a user mistake choosing image size that large. the default can be thumb if not specified. i'll split this into different commit later.

@roidayan roidayan force-pushed the add_image_field_type branch 4 times, most recently from 6a0b1c7 to 6dad5a8 Compare October 1, 2016 11:51
@roidayan roidayan force-pushed the add_image_field_type branch from 6dad5a8 to c453b20 Compare October 5, 2016 10:55
@roidayan
Copy link
Contributor Author

roidayan commented May 2, 2020

hi @tareq1988 , too bad this didn't get merged. I would like to revive this. we talked about user attaching raw img and code was updated to show thumbnail is exists.
i see a rebase is needed so i'll do that after some comments.

The image field type stores media id.
The media manager is limited to type image.
Settings section shows a thumbnail preview.

Signed-off-by: Roi Dayan <[email protected]>
@roidayan roidayan force-pushed the add_image_field_type branch from c453b20 to 00c2ea7 Compare May 2, 2020 15:50
@tomphilpotts
Copy link

@roidayan and @tareq1988 just tested this code within my project and all in working, great. Thank @roidayan

Definitely prefer the image field over the file, easier for my client to see what they have attached.

If you need help maintaining the repo will be happy to help, I see there are a few pull requests outstanding.

@roidayan
Copy link
Contributor Author

roidayan commented Feb 4, 2021

thanks. i'm still using this plugin as well. great thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants