User feedback adding recursive folders take 2 #4399

mifi · 2023-04-04T14:11:49Z

remake of #4388

Murderlon · 2023-04-11T09:44:03Z

packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx

@@ -280,6 +280,7 @@ export default class ProviderView extends View {
            if (!this.plugin.uppy.checkIfFileAlreadyExists(id)) {
              newFiles.push(fileInFolder)
              numNewFiles++
+              this.setLoading(this.plugin.uppy.i18n('addedNumFiles', { numFiles: numNewFiles }))


Why do we pass it to setLoading? Can't we inline the i18n in JSX?

loading is now a variable that can have three states, false, true or a String. if it's a string we show the string. Maybe not the cleanest way but I couldn't think of any prettier way to do it. Not sure what you mean by inlining i18n in JSX

In the UI component, can't we do something like this

{loading ? this.plugin.uppy.i18n('addedNumFiles', { numFiles: numNewFiles }) : null}

And only have setLoading as boolean toggle?

Yes but then we need to have one loading state variable to indicate loading without text and another state variable for loading with text. And we would have to have another state variable for numNewFiles i think?

Can't we always show it? When wouldn't we want to show it?

I thought it doesn't make any sense to show it when we are simply adding a list of files, or when the user browses through folders, I don't think it makes sense to show "adding X files", e.g. this function:

uppy/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx

Line 105 in d67a4a5

async getFolder (id, name) {

Murderlon

Not too happy with the boolean/string switching with setLoading but also don't know something better right off the bat. Can you add a todo comment for it, explaining what it does and that a better pattern is ideally needed?

mifi · 2023-04-13T17:05:47Z

I agree it’s not optimal. If for example i18n language changes while a message string has been set in the state, that string will not be updated to follow the new language. Can add a todo

| Package | Version | Package | Version | | ------------------------- | ------- | ------------------------- | ------- | | @uppy/angular | 0.5.2 | @uppy/progress-bar | 3.0.2 | | @uppy/audio | 1.1.1 | @uppy/provider-views | 3.3.0 | | @uppy/aws-s3 | 3.1.0 | @uppy/react | 3.1.2 | | @uppy/aws-s3-multipart | 3.2.0 | @uppy/react-native | 0.5.1 | | @uppy/box | 2.1.1 | @uppy/redux-dev-tools | 3.0.2 | | @uppy/companion | 4.5.0 | @uppy/remote-sources | 1.0.3 | | @uppy/companion-client | 3.1.3 | @uppy/screen-capture | 3.1.1 | | @uppy/compressor | 1.0.2 | @uppy/status-bar | 3.1.1 | | @uppy/core | 3.2.0 | @uppy/store-default | 3.0.3 | | @uppy/dashboard | 3.4.0 | @uppy/store-redux | 3.0.3 | | @uppy/drag-drop | 3.0.2 | @uppy/svelte | 3.0.2 | | @uppy/dropbox | 3.1.1 | @uppy/thumbnail-generator | 3.0.3 | | @uppy/facebook | 3.1.1 | @uppy/transloadit | 3.1.3 | | @uppy/file-input | 3.0.2 | @uppy/tus | 3.1.0 | | @uppy/form | 3.0.2 | @uppy/unsplash | 3.2.1 | | @uppy/golden-retriever | 3.0.3 | @uppy/url | 3.3.1 | | @uppy/google-drive | 3.1.1 | @uppy/utils | 5.3.0 | | @uppy/image-editor | 2.1.2 | @uppy/vue | 1.0.2 | | @uppy/informer | 3.0.2 | @uppy/webcam | 3.3.1 | | @uppy/instagram | 3.1.1 | @uppy/xhr-upload | 3.2.0 | | @uppy/locales | 3.2.0 | @uppy/zoom | 2.1.1 | | @uppy/onedrive | 3.1.1 | uppy | 3.8.0 | - @uppy/companion: increase max limits for remote file list operations (Mikael Finstad / #4417) - @uppy/xhr-upload: fix type in README.md (Top Master / #4416) - @uppy/core: improve performance of validating & uploading files (Mikael Finstad / #4402) - @uppy/provider-views: Concurrent file listing (Mikael Finstad / #4401) - @uppy/core,@uppy/locales,@uppy/provider-views: User feedback adding recursive folders take 2 (Mikael Finstad / #4399) - @uppy/dashboard: Single File Mode: fix layout and make optional (Artur Paikin / #4374) - @uppy/informer: add a check in `TransitionGroup` when component is null (Juan Belej / #4410) - meta: Fix logos in all the readmes (Artur Paikin / #4407) - meta: fix logo in readme (Kid / #4403)

| Package | Version | Package | Version | | ------------------------- | ------- | ------------------------- | ------- | | @uppy/angular | 0.5.2 | @uppy/progress-bar | 3.0.2 | | @uppy/audio | 1.1.1 | @uppy/provider-views | 3.3.0 | | @uppy/aws-s3 | 3.1.0 | @uppy/react | 3.1.2 | | @uppy/aws-s3-multipart | 3.2.0 | @uppy/react-native | 0.5.1 | | @uppy/box | 2.1.1 | @uppy/redux-dev-tools | 3.0.2 | | @uppy/companion | 4.5.0 | @uppy/remote-sources | 1.0.3 | | @uppy/companion-client | 3.1.3 | @uppy/screen-capture | 3.1.1 | | @uppy/compressor | 1.0.2 | @uppy/status-bar | 3.1.1 | | @uppy/core | 3.2.0 | @uppy/store-default | 3.0.3 | | @uppy/dashboard | 3.4.0 | @uppy/store-redux | 3.0.3 | | @uppy/drag-drop | 3.0.2 | @uppy/svelte | 3.0.2 | | @uppy/dropbox | 3.1.1 | @uppy/thumbnail-generator | 3.0.3 | | @uppy/facebook | 3.1.1 | @uppy/transloadit | 3.1.3 | | @uppy/file-input | 3.0.2 | @uppy/tus | 3.1.0 | | @uppy/form | 3.0.2 | @uppy/unsplash | 3.2.1 | | @uppy/golden-retriever | 3.0.3 | @uppy/url | 3.3.1 | | @uppy/google-drive | 3.1.1 | @uppy/utils | 5.3.0 | | @uppy/image-editor | 2.1.2 | @uppy/vue | 1.0.2 | | @uppy/informer | 3.0.2 | @uppy/webcam | 3.3.1 | | @uppy/instagram | 3.1.1 | @uppy/xhr-upload | 3.2.0 | | @uppy/locales | 3.2.0 | @uppy/zoom | 2.1.1 | | @uppy/onedrive | 3.1.1 | uppy | 3.8.0 | - @uppy/companion: increase max limits for remote file list operations (Mikael Finstad / transloadit#4417) - @uppy/xhr-upload: fix type in README.md (Top Master / transloadit#4416) - @uppy/core: improve performance of validating & uploading files (Mikael Finstad / transloadit#4402) - @uppy/provider-views: Concurrent file listing (Mikael Finstad / transloadit#4401) - @uppy/core,@uppy/locales,@uppy/provider-views: User feedback adding recursive folders take 2 (Mikael Finstad / transloadit#4399) - @uppy/dashboard: Single File Mode: fix layout and make optional (Artur Paikin / transloadit#4374) - @uppy/informer: add a check in `TransitionGroup` when component is null (Juan Belej / transloadit#4410) - meta: Fix logos in all the readmes (Artur Paikin / transloadit#4407) - meta: fix logo in readme (Kid / transloadit#4403)

mifi added 2 commits April 4, 2023 23:09

show how many files are added when loading

eff6876

remake of #4388

add french (cherry pick)

d67a4a5

mifi mentioned this pull request Apr 4, 2023

@uppy/provider-views: show how many files are added #4388

Closed

Murderlon reviewed Apr 11, 2023

View reviewed changes

Murderlon approved these changes Apr 13, 2023

View reviewed changes

add todo

07e704d

mifi merged commit 465dec7 into main Apr 15, 2023

mifi deleted the user-feedback-adding-recursive-folders-take-2 branch April 15, 2023 15:21

github-actions bot mentioned this pull request Apr 18, 2023

Release: [email protected] #4418

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User feedback adding recursive folders take 2 #4399

User feedback adding recursive folders take 2 #4399

mifi commented Apr 4, 2023

Murderlon Apr 11, 2023

mifi Apr 11, 2023

Murderlon Apr 11, 2023

mifi Apr 12, 2023 •

edited

Loading

Murderlon Apr 12, 2023

mifi Apr 12, 2023

Murderlon left a comment

mifi commented Apr 13, 2023

User feedback adding recursive folders take 2 #4399

User feedback adding recursive folders take 2 #4399

Conversation

mifi commented Apr 4, 2023

Murderlon Apr 11, 2023

Choose a reason for hiding this comment

mifi Apr 11, 2023

Choose a reason for hiding this comment

Murderlon Apr 11, 2023

Choose a reason for hiding this comment

mifi Apr 12, 2023 • edited Loading

Choose a reason for hiding this comment

Murderlon Apr 12, 2023

Choose a reason for hiding this comment

mifi Apr 12, 2023

Choose a reason for hiding this comment

Murderlon left a comment

Choose a reason for hiding this comment

mifi commented Apr 13, 2023

mifi Apr 12, 2023 •

edited

Loading