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

core: setOptions for Core and plugins #1728

Merged
merged 20 commits into from
Nov 4, 2019
Merged

Conversation

arturi
Copy link
Contributor

@arturi arturi commented Jul 12, 2019

  • Adds an uppy.setOptions({ restrictions: { maxNumberOfFiles: 3 } }) — we’ve changed maxNumberOfFiles depending on some condition in our app on the fly
  • Adds an uppy.getPlugin('Dashboard').setOptions({ note: 'You can only upload 3 files' }) — we’ve changed the note to reflect the new maxNumberOfFiles
  • Adds an uppy.getPlugin('Dashboard').i18nInit() API to re-init the i18n translations, so that new stings are used after they’ve been updated with methods above. — done automatically now

We keep getting requests to update options or change locales in uppy on the fly, this PR addresses that (and fixes #1687):

Is there a way to change Uppy Instance files restrictions, with a dynamic function like uppy.setState for example ?

I would like to dynamically change the max upload files number

ToDo:

  • add i18nInit to all plugins that use translation strings
  • tests
  • docs, with a note that some options won’t take effect, like for XHR, or when an upload is in progress

Copy link
Contributor

@goto-bus-stop goto-bus-stop left a comment

Choose a reason for hiding this comment

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

this is probably a good direction .. if there is some option that cannot be changed without breaking everything, we could override setOptions() to blacklist it. For some options like limit, we could add a method to the RateLimitedQueue to change the limit on the fly.

packages/@uppy/dashboard/src/index.js Show resolved Hide resolved
@goto-bus-stop
Copy link
Contributor

goto-bus-stop commented Aug 7, 2019

  • Adds an uppy.setOptions({ restrictions: { maxNumberOfFiles: 3 } }) — we’ve changed maxNumberOfFiles depending on some condition in our app on the fly

Regarding this, we probably need to merge in the restrictions object too then right? Else you lose any other restrictions. Maybe we should move all the merging stuff that we do in the constructor into setOptions() as well, and then use this.setOptions(opts) in the constructor?

@BallisticPain
Copy link

Would this allow for changing the size of the Uppy instance? I would love to save screen space by having a thing drag n drop window that expands or grows larger to encompass the thumbnail(s) with their respective controls, etc.

This would require a re-draw so this may need an "event" to trigger after setting?

@arturi
Copy link
Contributor Author

arturi commented Oct 18, 2019

  • Adds an uppy.setOptions({ restrictions: { maxNumberOfFiles: 3 } }) — we’ve changed maxNumberOfFiles depending on some condition in our app on the fly

Regarding this, we probably need to merge in the restrictions object too then right? Else you lose any other restrictions.

Then I wonder if we should merge all objects manually, like meta will also get overwritten. Or we could document the fact that you need to do setOptions({ restrictions: ...uppy.opts.restrictions, { minNumberOfFiles: 2 }), and merge nothing manually. This is not pleasant, but consistent?

@arturi
Copy link
Contributor Author

arturi commented Oct 18, 2019

Also not sure if we should allow changing locale per plugin via setOptions, when you can set it in Core and that would already override the per-plugin setting.

@arturi
Copy link
Contributor Author

arturi commented Oct 18, 2019

@BallisticPain if I understand you correctly, and you’d like to dynamically change the size of @uppy/dashboard or @uppy/drag-drop, then indeed you’ll be able to just that with uppy.getPlugin('Dashboard').setOptions({width: 300}) and it will re-render automagically!

@arturi arturi marked this pull request as ready for review October 23, 2019 17:51
Copy link
Contributor

@goto-bus-stop goto-bus-stop left a comment

Choose a reason for hiding this comment

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

Seems almost perfect! One last thing: when you change the locale in Core, all the plugins' locales need to be updated as well, so the things they inherited are reflected. For ex, add this to ./examples/dev/Dashboard.js:

setTimeout(() => {
  uppy.setOptions({
    locale: require('@uppy/locales/src/th_TH')
  })
}, 2000)

after 2s the main dashboard screens will still be in english. if you open a panel (like for GDrive), it's in Thai. i'm not sure exactly why the panel is in Thai and the main screens are not. but it's fixed by refreshing the Translator instance the dashboard uses with dashboardPlugin.i18nInit().

@arturi
Copy link
Contributor Author

arturi commented Oct 30, 2019

Thanks, Renée!

i'm not sure exactly why the panel is in Thai and the main screens are not

Simple: ProviderViews are using locale strings form Core ;)

@goto-bus-stop goto-bus-stop merged commit 4e54483 into master Nov 4, 2019
@goto-bus-stop goto-bus-stop deleted the feature/set-options branch November 4, 2019 09:33
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.

Change Uppy object configuration on flight
3 participants