-
Notifications
You must be signed in to change notification settings - Fork 2.1k
@uppy/dashboard: fix metafield form validation #3113
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
Conversation
Browsers without support for the HTML `form` attribute would misbehave, making it impossible to validate the form at all, and potentially validate an unrelated upper form if users are embeding Uppy inside a a `<form>`. Fixes: transloadit#3111
@@ -154,6 +166,7 @@ class FileCard extends Component { | |||
<button | |||
className="uppy-u-reset uppy-c-btn uppy-c-btn-primary uppy-Dashboard-FileCard-actionsBtn" | |||
type="submit" | |||
in HTMLButtonElement.prototype ? undefined : this.handleSave} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might be a case where a comment with some context will help future contributors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of bd7ca31? Open to better suggestions if that do not suffice.
* @uppy/dashboard: fix metafield form validation Browsers without support for the HTML `form` attribute would misbehave, making it impossible to validate the form at all, and potentially validate an unrelated upper form if users are embeding Uppy inside a a `<form>`. Fixes: #3111 * Add comments * Fix docs and types
Bug introduced in #2896, fix in #3113 didn’t work for uppy 1.x, because Preact 8 ignores the form attribute. We’ve removed the use of form for 1.x, so you still get the required meta field functionality, but no feedback from the browser Co-Authored-By: Antoine du Hamel <14309773+aduh95@users.noreply.github.com>
* main: (23 commits) Release Set node version in `workflows/cdn.yml` to 16.x Release build: add stylelint (#3124) Core: rename allowMultipleUploads to allowMultipleUploadBatches (#3115) meta: enforce `no-unused-vars` linter rule (#3118) writing-plugins: update example to use `i18nInit` (#3122) @uppy/core: reject empty string as valid value for required meta fields (#3119) Safely escape <script> injected code in companion `send-token.js` (#3101) @uppy/dashboard: fix metafield form validation (#3113) Clean up `BACKLOG.md` & add Vimeo as todo Add referrer to transloadit.com link (#3116) @uppy/locales latest version is 1.22.0 🙈 Stricter linter (#3095) @uppy/aws-s3: refactor to use private fields (#3094) build: fix legacy bundle (#3112) Fix locales — point to CDN v1.31.0 Fix typo in `docs/companion.md` Changelog for 1.31.0 and patches Strictly type uppy events (#3085) ...
* @uppy/dashboard: fix metafield form validation Browsers without support for the HTML `form` attribute would misbehave, making it impossible to validate the form at all, and potentially validate an unrelated upper form if users are embeding Uppy inside a a `<form>`. Fixes: transloadit#3111 * Add comments * Fix docs and types
Bug introduced in transloadit#2896, fix in transloadit#3113 didn’t work for uppy 1.x, because Preact 8 ignores the form attribute. We’ve removed the use of form for 1.x, so you still get the required meta field functionality, but no feedback from the browser Co-Authored-By: Antoine du Hamel <14309773+aduh95@users.noreply.github.com>
Browsers without support for the HTML
form
attribute would misbehave, making it impossible to validate the form at all, and potentially validate an unrelated upper form if users are embeding Uppy inside a a<form>
.FWIW this attribute is supported on all the browsers we officially support, but it's quite cheap to add a check and fallback to the old behavior if we don't detect any support.
It would be worth to backport this to Uppy 1.x, as we still support IE 11 there. There might also be another bug related to Preact 8 on that branch, as it seems the
form
attribute is not added at all to the DOM elements, so we should do some extra check before releasing a patch version.Fixes: #3111