Skip to content

Conversation

xlvisuals
Copy link

  • fixed component not emitting 'change' when changing file
  • added support for file extension as mime type (as per spec)

dependabot bot and others added 7 commits October 19, 2023 11:23
Bumps [@babel/traverse](https://github.com/babel/babel/tree/HEAD/packages/babel-traverse) from 7.22.8 to 7.23.2.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.23.2/packages/babel-traverse)

---
updated-dependencies:
- dependency-name: "@babel/traverse"
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [browserify-sign](https://github.com/crypto-browserify/browserify-sign) from 4.2.1 to 4.2.2.
- [Changelog](https://github.com/browserify/browserify-sign/blob/main/CHANGELOG.md)
- [Commits](browserify/browserify-sign@v4.2.1...v4.2.2)

---
updated-dependencies:
- dependency-name: browserify-sign
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.15.2 to 1.15.4.
- [Release notes](https://github.com/follow-redirects/follow-redirects/releases)
- [Commits](follow-redirects/follow-redirects@v1.15.2...v1.15.4)

---
updated-dependencies:
- dependency-name: follow-redirects
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
…npm_and_yarn/babel/traverse-7.23.2

Bump @babel/traverse from 7.22.8 to 7.23.2
…npm_and_yarn/browserify-sign-4.2.2

Bump browserify-sign from 4.2.1 to 4.2.2
…npm_and_yarn/follow-redirects-1.15.4

Bump follow-redirects from 1.15.2 to 1.15.4
- fixed component not emitting 'change' when changing file
- added support for file extension as mime type (as per <input> spec)
Copy link

codesandbox bot commented Jul 18, 2025

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Improved support for file extensions as accepted file types.
Specify any or all file type as extension (e.g. .pdf) instead of their mimetype.
@alessiomaffeis
Copy link
Owner

Hi @xlvisuals! First of all, thanks for opening this PR. 🙏

  • Re: the change emit issue, I've tested your fix, and I've found more issues around the original logic (probably regressions from earlier PRs). I've spent some time and I've pushed a new version that should fix all the issues related to the change and prefill events. Now it should behave like described in the README. Please test v4.0.2, and let me know if you still encounter problems pertaining the change events (I've published a major version even if it was really just a fix as it might break existing apps that worked around the previous issues. I've also taken the chance to make Vue 2.7 the minimum supported version)

  • Re: the mime type fix, what are you trying to achieve exactly?

@xlvisuals
Copy link
Author

Hi! Thanks a lot for looking into this!

I briefly tested your new version and found two issues:

  1. Image format support
    Version 4.0.2 works great for uploading image types that the browser can preview - jpg, png, gif, etc.
    It does not work for formats where preview is not supported, for example image/tiff.
    No 'change' event is emitted for these types.

I don't understand the logic/benefit of emitting 'change' in onFileChange() and loadImage() and to tie it up with the 'preview' event. I feel like 'change' and 'preview' are unrelated and the else clause is causing issues here.
I would suggest only to emit 'change' only in onFileChange(), and do it every time unless 'error' is emitted first.

  1. Accept property / file type support
    The accept attribute for elements allow mimetypes as well as file extensions.
    See https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/input/file#accept

vue-picture-input, including v4.0.2, only considers mimetypes and not file extensions. That means you can't upload uncommon file types.

This can be easily fixed by replacing line 375

if (this.fileTypes.indexOf(this.fileType) === -1) { ... }

with

const fileExtension = '.' + this.fileName.split('.').pop()
if (this.fileTypes.indexOf(this.fileType) === -1 &&
   this.fileTypes.indexOf(fileExtension) === -1) { ... }

This allows users to specify extensions (e.g. ".pdf") as well as mimetypes for the accept property.

@alessiomaffeis
Copy link
Owner

Version 4.0.2 works great for uploading image types that the browser can preview - jpg, png, gif, etc.
It does not work for formats where preview is not supported, for example image/tiff.

I tried with a TIFF image and it worked correctly with preview too. Can you share an image where it doesn't work? (Also, which browser+version?). But yeah, a failure in rendering the preview should not prevent the change event to be fired, provided that it's a valid image.

I don't understand the logic/benefit of emitting 'change' in onFileChange() and loadImage() and to tie it up with the 'preview' event. I feel like 'change' and 'preview' are unrelated and the else clause is causing issues here.
I would suggest only to emit 'change' only in onFileChange(), and do it every time unless 'error' is emitted first.

Yeah, that logic is really convoluted, I had troubles following too after all this time, and it could definitely benefit from a refactoring. Anyway, it now should behave as intended, from my testing, which means:

  • the change event is emitted both when the image is selected through file browser or by drag and drop
  • the prefill event is emitted only when the prefill image is loaded and shown in the preview.

I want to also note that prefilling an image is for displaying a placeholder preview only—it doesn't actually change the file of the underlying input, that's never been the case (I will clarify that in the README, as it's been a cause of misunderstanding for other people).

Regarding the file type support: got it. Could you open a separate PR just for that, @xlvisuals? I will include it in the next release!

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