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/react: remove react: prefix from id & allow id as a prop #5228

Merged
merged 2 commits into from
Jun 10, 2024

Conversation

Murderlon
Copy link
Member

Fixes #5218

Also removes the confusing react: prefix, which serves no purpose and is confusing coming from the vanilla version.

@Murderlon Murderlon requested review from mifi and aduh95 June 4, 2024 16:31
@Murderlon Murderlon self-assigned this Jun 4, 2024
Copy link
Contributor

github-actions bot commented Jun 4, 2024

Diff output files
diff --git a/packages/@uppy/react/lib/Dashboard.js b/packages/@uppy/react/lib/Dashboard.js
index 04517e8..df232eb 100644
--- a/packages/@uppy/react/lib/Dashboard.js
+++ b/packages/@uppy/react/lib/Dashboard.js
@@ -29,9 +29,9 @@ class Dashboard extends Component {
       uppy,
       ...options
     } = {
-      id: "react:Dashboard",
-      inline: true,
+      id: "Dashboard",
       ...this.props,
+      inline: true,
       target: this.container,
     };
     uppy.use(DashboardPlugin, options);
diff --git a/packages/@uppy/react/lib/DashboardModal.js b/packages/@uppy/react/lib/DashboardModal.js
index f335860..3bd2812 100644
--- a/packages/@uppy/react/lib/DashboardModal.js
+++ b/packages/@uppy/react/lib/DashboardModal.js
@@ -44,8 +44,8 @@ class DashboardModal extends Component {
       ...rest
     } = this.props;
     const options = {
+      id: "DashboardModal",
       ...rest,
-      id: "react:DashboardModal",
       inline: false,
       target,
       open,
diff --git a/packages/@uppy/react/lib/DragDrop.js b/packages/@uppy/react/lib/DragDrop.js
index 4bfa1a2..8519fc3 100644
--- a/packages/@uppy/react/lib/DragDrop.js
+++ b/packages/@uppy/react/lib/DragDrop.js
@@ -32,9 +32,10 @@ class DragDrop extends Component {
       width,
       height,
       note,
+      id,
     } = this.props;
     const options = {
-      id: "react:DragDrop",
+      id: id || "DragDrop",
       locale,
       inputName,
       width,
diff --git a/packages/@uppy/react/lib/FileInput.js b/packages/@uppy/react/lib/FileInput.js
index 3451e6a..5cb6e66 100644
--- a/packages/@uppy/react/lib/FileInput.js
+++ b/packages/@uppy/react/lib/FileInput.js
@@ -19,9 +19,10 @@ class FileInput extends Component {
       locale,
       pretty,
       inputName,
+      id,
     } = this.props;
     const options = {
-      id: "react:FileInput",
+      id: id || "FileInput",
       locale,
       pretty,
       inputName,
diff --git a/packages/@uppy/react/lib/ProgressBar.js b/packages/@uppy/react/lib/ProgressBar.js
index 3d0e39d..6d45962 100644
--- a/packages/@uppy/react/lib/ProgressBar.js
+++ b/packages/@uppy/react/lib/ProgressBar.js
@@ -29,9 +29,10 @@ class ProgressBar extends Component {
       uppy,
       fixed,
       hideAfterFinish,
+      id,
     } = this.props;
     const options = {
-      id: "react:ProgressBar",
+      id: id || "ProgressBar",
       fixed,
       hideAfterFinish,
       target: this.container,
diff --git a/packages/@uppy/react/lib/StatusBar.js b/packages/@uppy/react/lib/StatusBar.js
index cd96dc3..306dec4 100644
--- a/packages/@uppy/react/lib/StatusBar.js
+++ b/packages/@uppy/react/lib/StatusBar.js
@@ -34,9 +34,10 @@ class StatusBar extends Component {
       showProgressDetails,
       hideAfterFinish,
       doneButtonHandler,
+      id,
     } = this.props;
     const options = {
-      id: "react:StatusBar",
+      id: id || "StatusBar",
       hideUploadButton,
       hideRetryButton,
       hidePauseResumeButton,

packages/@uppy/react/src/DragDrop.ts Outdated Show resolved Hide resolved
packages/@uppy/react/src/DragDrop.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

I'd be surprised if that prefix really served no purpose, it was introduced in #776, unfortunately there's no explanation on that PR

@Murderlon
Copy link
Member Author

I'd be very surprised if it did. It's probably from falsely assuming that Dashboard already exists, as we load the vanilla version inside the React component, but that doesn't matter. Only one plugin is registered.

Comment on lines +54 to +56
id: 'Dashboard',
...this.props,
inline: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

This removes the possibility to override inline via props, is it on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's on purpose and documented in #5206

@Murderlon Murderlon merged commit f72aed7 into 4.x Jun 10, 2024
17 checks passed
@Murderlon Murderlon deleted the react-ids branch June 10, 2024 13:43
github-actions bot added a commit that referenced this pull request Jun 13, 2024
| Package              |       Version | Package              |       Version |
| -------------------- | ------------- | -------------------- | ------------- |
| @uppy/aws-s3         |  4.0.0-beta.6 | @uppy/react          |  4.0.0-beta.6 |
| @uppy/locales        |  4.0.0-beta.3 | @uppy/transloadit    |  4.0.0-beta.8 |
| @uppy/provider-views |  4.0.0-beta.8 | uppy                 | 4.0.0-beta.11 |

- docs: clarify assemblyOptions for @uppy/transloadit (Merlijn Vos / #5226)
- @uppy/react: remove `react:` prefix from `id` & allow `id` as a prop (Merlijn Vos / #5228)
- docs: correct allowedMetaFields (Merlijn Vos / #5227)
- docs: remove `extraData` note from migration guide (Mikael Finstad / #5219)
- meta: fix AWS test suite (Antoine du Hamel / #5229)




| Package              | Version | Package              | Version |
| -------------------- | ------- | -------------------- | ------- |
| @uppy/locales        |   3.5.4 | @uppy/transloadit    |   3.7.1 |
| @uppy/provider-views |  3.12.1 | uppy                 |  3.26.1 |

- meta: Improve aws-node example readme (Artur Paikin / #4753)
- @uppy/locales: Added translation string (it_IT) (Samuel / #5237)
- @uppy/transloadit: fix transloadit:result event (Merlijn Vos / #5231)
- @uppy/provider-views: fix wrong font for files (Merlijn Vos / #5234)
Murderlon added a commit that referenced this pull request Jun 17, 2024
* 4.x:
  Renames & `eslint-disable react/require-default-props` removal (#5251)
  coalesce options `bucket` and `getKey` (#5169)
  @uppy/aws-s3: add `endpoint` option (#5173)
  @uppy/locales: fix `fa_IR` export (#5241)
  improve companion logging (#5250)
  Release: uppy@4.0.0-beta.11 (#5243)
  @uppy/core: add generic to `getPlugin` (#5247)
  docs: add 4.x migration guide (#5206)
  @uppy/transloadit: also fix outdated assembly transloadit:result (#5246)
  docs - fix typo in the url
  @uppy/core: set default for Body generic (#5244)
  Release: uppy@3.26.1 (#5242)
  docs: clarify assemblyOptions for @uppy/transloadit (#5226)
  meta: Improve aws-node example readme (#4753)
  @uppy/react: remove `react:` prefix from `id` & allow `id` as a prop (#5228)
  Added translation string (it_IT) (#5237)
  docs: correct allowedMetaFields (#5227)
  @uppy/transloadit: fix transloadit:result event (#5231)
  docs: remove `extraData` note from migration guide (#5219)
  @uppy/provider-views: fix wrong font for files (#5234)
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.

3 participants