Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix: input file overwrite in merge #2335

Merged

Conversation

omar-ahmed42
Copy link
Contributor

@omar-ahmed42 omar-ahmed42 commented Nov 26, 2024

Description

Problem:

Each time a new file input operation takes place, all the existing file(s) are replaced with the newly selected file(s).

Reason:

When a user tries to upload a file using <input> by selecting the file(s) from the file system, a "change" event is triggered, this event should be handled by appending newly selected files to existing ones and adding it as well to element.files (the input element's files).

Solution:

  1. Differentiating between drag and drop files and input uploaded approaches using "source" in event.detail.source (to handle each of them accordingly).
  2. After appending the necessary files to the input element and allFiles, dispatch a "file-input-change" event to allow other components/functions/files/modules to perform their operations on all of the selected files (after appending upload/drag-drop files).

Closes #2285

Checklist

  • I have read the Contribution Guidelines
  • I have performed a self-review of my own code
  • I have attached images of the change if it is UI based
  • I have commented my code, particularly in hard-to-understand areas
  • If my code has heavily changed functionality I have updated relevant docs on Stirling-PDFs doc repo
  • My changes generate no new warnings
  • I have read the section Add New Translation Tags (for new translation tags only)

- Fix a bug that caused existing selected/uploaded files to be overwritten when a new input file is uploaded through input element.
- Add source property to change event to differentiate between uploaded files using input element and drag/drop uploads to avoid processing drag/drop files more than once, thus avoiding file duplication (file duplication resulting from copying drop/drop files to input files on each 'change' event).
- Dispatch "file-input-change" event after each "change" event in file upload, to notify other functions/components relying on the files provided by the \<input\> element.
- Use "file-input-change" instead of "change" event to display the latest version of uploaded files.

# FAQ:
- Why use "file-input-change" instead of "change" in merge.js?
= "change" event is automatically triggered when a file is uploaded through \<input\> element which would replace all the existing selected/uploaded files including the drag/drop files.

## Example:
Let's say that the user wants to upload/select the x.pdf, y.pdf and z.pdf all together:

- user selects "x.pdf" -> file selected successfully.
= selected files: x.pdf

- user drags and drops "y.pdf" -> file dropped successfully
= selected files: x.pdf, y.pdf

- user selects again using \<input\> "z.pdf" -> file selected succesfully overwriting selected files.
= selected files: z.pdf
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. Bug Something isn't working labels Nov 26, 2024
@github-actions github-actions bot added Front End Issues or pull requests related to front-end development and removed Bug Something isn't working labels Nov 26, 2024
@Frooodle Frooodle enabled auto-merge (squash) November 26, 2024 20:39
@Frooodle Frooodle merged commit 654bc94 into Stirling-Tools:main Nov 26, 2024
6 checks passed
@reecebrowne
Copy link
Contributor

/deploypr

Copy link
Contributor

🚀 PR Test Deployment

Your PR has been deployed for testing!

🔗 Test URL: http://185.252.234.121:2335

This deployment will be automatically cleaned up when the PR is closed.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Front End Issues or pull requests related to front-end development size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: merge-pdfs error with different folder source
3 participants