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

Adding images to the page as they are uploaded #2

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

Conversation

felipecsl
Copy link

When you're uploading images, it is very likely that you wanna display them to the user right away.
I just realized as I am typing this, that the uploader can be used for other types of files as well, but if you are uploading images, this is very convenient and will put image tags in the page with thumbnails of the images you just uploaded. Since not necessarily the files will be images, we might need to think whether or not it is desirable to have this feature. Anyway, I had to write it, so if you think it can be useful, let me know :)

@ncri
Copy link
Owner

ncri commented Apr 8, 2012

Hi, this looks good! However, maybe we better keep it as a separate fork, to keep the example basic? You could add a link to your fork to the readme though. It would also be good to support non image files, as you suggest. Maybe you could display an icon (or just the filename) for non image files. What do you think?

@felipecsl
Copy link
Author

I like the idea of displaying an icon + filename for non images
We could have a basic set of icon for, let's say, pdf, doc, txt, zip, etc. and a generic one for all other types, like a gear, for example. Below we could show the filename as well.

This should be easy to do, I can include in the pull request, but I think the question is, do you think it makes sense to include it in the project or you rather keep it separated?

@ncri
Copy link
Owner

ncri commented Apr 9, 2012

Sounds good. Well, I think we could include it. But can you add some comments to document the js in the success callback? Thanks!

@felipecsl
Copy link
Author

Sure, will do. Thanks!

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.

2 participants