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

@uppy/utils: better fallbacks for the drag & drop API #4260

Merged
merged 1 commit into from
Jan 26, 2023

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jan 3, 2023

…ag&drop

@Murderlon Murderlon changed the title @uppy/utils: be more defensive when falling back to the old API on dr… @uppy/utils: better fallbacks for the drag & drop API Jan 3, 2023
@Murderlon Murderlon linked an issue Jan 3, 2023 that may be closed by this pull request
2 tasks
@aduh95 aduh95 added the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label Jan 3, 2023
@github-actions github-actions bot removed pending end-to-end tests safe to test Add this label on trustworthy PRs to spawn the e2e test suite labels Jan 3, 2023
// For each dropped item, - make sure it's a file/directory, and start deepening in!
if (entry.kind === 'file') {
const file = await entry.getFile()
if (file !== null) {
file.relativePath = relativePath ? `${relativePath}/${entry.name}` : null
yield file
}
} else if (lastResortFile != null) yield lastResortFile
} else if (entry.kind === 'directory') {
for await (const handle of entry.values()) {
yield* createPromiseToAddFileOrParseDirectory(handle, `${relativePath}/${entry.name}`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we using generator and yield here? Not saying we shouldn't, but it’s hard to read and I think this is the only place we use them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"hard to read" is subjective, I kinda doubt this would be more readable without using generators – not saying that this code is the most readable, but rather than we could (should?) make it way more readable while keeping generators. In my experience using async generators is the "best" way to iterate through a file tree, as in it's probably the most memory efficient while being the more elegant solution.

@arturi arturi self-requested a review January 17, 2023 23:08
@ChristianVermeulen
Copy link

We are having quite a few customers complain about this issue so we are very very keen on seeing this merged and fixed! 🤟

@arturi arturi merged commit 79bab48 into transloadit:main Jan 26, 2023
@github-actions github-actions bot mentioned this pull request Jan 26, 2023
github-actions bot added a commit that referenced this pull request Jan 26, 2023
| Package                | Version | Package                | Version |
| ---------------------- | ------- | ---------------------- | ------- |
| @uppy/audio            |   1.0.3 | @uppy/locales          |   3.0.5 |
| @uppy/aws-s3           |   3.0.5 | @uppy/react            |   3.1.0 |
| @uppy/aws-s3-multipart |   3.1.2 | @uppy/react-native     |   0.5.0 |
| @uppy/companion        |   4.2.0 | @uppy/transloadit      |   3.1.0 |
| @uppy/core             |   3.0.5 | @uppy/utils            |   5.1.2 |
| @uppy/dashboard        |   3.2.1 | uppy                   |   3.4.0 |

- @uppy/utils: better fallbacks for the drag & drop API (Antoine du Hamel / #4260)
- @uppy/core: fix metafields validation when used as function (Merlijn Vos / #4276)
- @uppy/companion: allow customizing express session prefix (Mikael Finstad / #4249)
- meta: Fix comment about COMPANION_PATH (Collin Allen / #4279)
- @uppy/companion: Fix typo in KUBERNETES.md (Collin Allen / #4277)
- @uppy/locales: update zh_TW.js (5idereal / #4270)
- meta: ci: make sure Yarn's global cache is disabled (Antoine du Hamel / #4268)
- @uppy/aws-s3-multipart: fix metadata shape (Antoine du Hamel / #4267)
- meta: example: add multipart support to `aws-nodejs` (Antoine du Hamel / #4257)
- @uppy/react-native: example: revive React Native example (Giacomo Cerquone / #4164)
- @uppy/utils: Fix getSpeed type (referenced `bytesTotal` instead of `uploadStarted`) (Pascal Wengerter / #4263)
- @uppy/companion: document how to run many instances (Mikael Finstad / #4227)
- @uppy/aws-s3-multipart: add support for `allowedMetaFields` option (Antoine du Hamel / #4215)
- meta: Fix indentation in generate-test.mjs (Youssef Victor / #4181)
- @uppy/react: deprecate `useUppy` (Merlijn Vos / #4223)
- meta: fix typo in README.md (Fuad Herac / #4254)
- meta: Don’t close stale issues automatically (Artur Paikin / #4246)
- meta: upgrade to Vite 4 and ESBuild 0.16 (Antoine du Hamel / #4243)
- @uppy/audio: @uppy/audio fix typo in readme (elliotsayes / #4240)
- @uppy/aws-s3: fix: add https:// to digital oceans link (Le Gia Hoang / #4165)
- website: Simplify Dashboard code sample (Artur Paikin / #4197)
- @uppy/transloadit: introduce `assemblyOptions`, deprecate other options (Merlijn Vos / #4059)
- @uppy/core: fix typo in Uppy.test.js (Ikko Ashimine / #4235)
- @uppy/aws-s3-multipart: fix singPart type (Stefan Schonert / #4224)
@aduh95 aduh95 deleted the drop-files-fallback branch January 26, 2023 16:46
@arturi
Copy link
Contributor

arturi commented Feb 6, 2023

@ChristianVermeulen hi, could you please confirm if the recent release Uppy 3.4 https://github.com/transloadit/uppy/releases/tag/uppy%403.4.0 has fixed the issue for you?

@ChristianVermeulen
Copy link

@ChristianVermeulen hi, could you please confirm if the recent release Uppy 3.4 uppy%403.4.0 (release) has fixed the issue for you?

I missed the notification about the merge. I'm upgrading as we speak, will get back to you asap!

@ChristianVermeulen
Copy link

ChristianVermeulen commented Feb 6, 2023

@arturi I ran into some issues while upgrading.. Been trying to debug this for over an hour but can't get it to compile my js now 😅.

Here is the bugreport: #4304

@ChristianVermeulen
Copy link

@arturi I can hereby confirm it has been fixed! Thanks you 💪

@arturi
Copy link
Contributor

arturi commented Feb 8, 2023

@ChristianVermeulen amazing, thanks!

HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
| Package                | Version | Package                | Version |
| ---------------------- | ------- | ---------------------- | ------- |
| @uppy/audio            |   1.0.3 | @uppy/locales          |   3.0.5 |
| @uppy/aws-s3           |   3.0.5 | @uppy/react            |   3.1.0 |
| @uppy/aws-s3-multipart |   3.1.2 | @uppy/react-native     |   0.5.0 |
| @uppy/companion        |   4.2.0 | @uppy/transloadit      |   3.1.0 |
| @uppy/core             |   3.0.5 | @uppy/utils            |   5.1.2 |
| @uppy/dashboard        |   3.2.1 | uppy                   |   3.4.0 |

- @uppy/utils: better fallbacks for the drag & drop API (Antoine du Hamel / transloadit#4260)
- @uppy/core: fix metafields validation when used as function (Merlijn Vos / transloadit#4276)
- @uppy/companion: allow customizing express session prefix (Mikael Finstad / transloadit#4249)
- meta: Fix comment about COMPANION_PATH (Collin Allen / transloadit#4279)
- @uppy/companion: Fix typo in KUBERNETES.md (Collin Allen / transloadit#4277)
- @uppy/locales: update zh_TW.js (5idereal / transloadit#4270)
- meta: ci: make sure Yarn's global cache is disabled (Antoine du Hamel / transloadit#4268)
- @uppy/aws-s3-multipart: fix metadata shape (Antoine du Hamel / transloadit#4267)
- meta: example: add multipart support to `aws-nodejs` (Antoine du Hamel / transloadit#4257)
- @uppy/react-native: example: revive React Native example (Giacomo Cerquone / transloadit#4164)
- @uppy/utils: Fix getSpeed type (referenced `bytesTotal` instead of `uploadStarted`) (Pascal Wengerter / transloadit#4263)
- @uppy/companion: document how to run many instances (Mikael Finstad / transloadit#4227)
- @uppy/aws-s3-multipart: add support for `allowedMetaFields` option (Antoine du Hamel / transloadit#4215)
- meta: Fix indentation in generate-test.mjs (Youssef Victor / transloadit#4181)
- @uppy/react: deprecate `useUppy` (Merlijn Vos / transloadit#4223)
- meta: fix typo in README.md (Fuad Herac / transloadit#4254)
- meta: Don’t close stale issues automatically (Artur Paikin / transloadit#4246)
- meta: upgrade to Vite 4 and ESBuild 0.16 (Antoine du Hamel / transloadit#4243)
- @uppy/audio: @uppy/audio fix typo in readme (elliotsayes / transloadit#4240)
- @uppy/aws-s3: fix: add https:// to digital oceans link (Le Gia Hoang / transloadit#4165)
- website: Simplify Dashboard code sample (Artur Paikin / transloadit#4197)
- @uppy/transloadit: introduce `assemblyOptions`, deprecate other options (Merlijn Vos / transloadit#4059)
- @uppy/core: fix typo in Uppy.test.js (Ikko Ashimine / transloadit#4235)
- @uppy/aws-s3-multipart: fix singPart type (Stefan Schonert / transloadit#4224)
G-Rath added a commit to ackama/nzsl-share that referenced this pull request Jul 26, 2024
G-Rath added a commit to ackama/nzsl-share that referenced this pull request Jul 26, 2024
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.

Drop Mails from Outlook does no longer work with Chromium 108
4 participants