Check upload limits before trying to upload large files #1876
Conversation
| } | ||
|
|
||
| uploadFiles(files) { | ||
| uploadFiles(files, limits) { |
There was a problem hiding this comment.
Yep, should remove.
| @@ -16,6 +16,7 @@ limitations under the License. | |||
| */ | |||
| import React from 'react'; | |||
| import PropTypes from 'prop-types'; | |||
| import filesize from "filesize"; | |||
| } | ||
|
|
||
| isFileUploadAllowed(file) { | ||
| if (this._uploadLimits.upload_size != null && file.size > this._uploadLimits.upload_size) { |
There was a problem hiding this comment.
!= null is slightly confusing here since you're relying on undefined == null (and of course 0 == null too, so a limit of 0 will mean unlimited). Probably better to say === undefined.
| }).catch(() => { | ||
| // HS can't or won't provide limits, so don't give any. | ||
| this._uploadLimits = {}; | ||
| }) |
There was a problem hiding this comment.
This will re-fetch the limits every time the composer is mounted, ie. every time you change rooms, which probably isn't what you wanted.
There was a problem hiding this comment.
I was struggling with this because I realised if we restarted the HS (or indeed, just the media server) then the limits become stale. Given it's a small request decided to make it fetch once each time we load it.
However I confess this is a lazy way to achieve that and I should probably hook onto something else.
|
Thanks for the feedback. Will wait on the spec proposal to be finalized before I continue. |
Improve upload dialog ux
Half-Shot
force-pushed
the
hs/upload-limits
branch
from
3948ca9 to
541f1d7
Compare
|
@Half-Shot I assume you plan on addressing the build failure and merge conflicts on this? It seems to look okay from my perspective otherwise. |
|
Ah yes, will do. |
|
@Half-Shot looks like you have test failures |
|
@turt2live I do, but I don't know how. The errors don't correlate with any of the things I've changed. EDIT: I never learn, always check the commas after resolving a JSON merge conflict. The PR still won't work because the JS-SDK doesn't support |
An explanation is in matrix-org/matrix-spec-proposals#1189.
Depends on #644 and (optionally) #3184.
Demo