Dashboard - convert some files to typescript - 2

[lakesare]

• converts AddFiles.tsx
• converts Dashboard.tsx

Only merge after #5381, #5380

[github-actions]

Diff output files

diff --git a/packages/@uppy/dashboard/lib/Dashboard.js b/packages/@uppy/dashboard/lib/Dashboard.js
index a4e8014..929c8e2 100644
--- a/packages/@uppy/dashboard/lib/Dashboard.js
+++ b/packages/@uppy/dashboard/lib/Dashboard.js
@@ -71,6 +71,7 @@ const defaultOptions = {
   autoOpen: null,
   disabled: false,
   disableLocalFiles: false,
+  nativeCameraFacingMode: "",
   doneButtonHandler: undefined,
   onRequestCloseModal: null,
 };
diff --git a/packages/@uppy/dashboard/lib/components/AddFiles.js b/packages/@uppy/dashboard/lib/components/AddFiles.js
index 8a0edf9..68d3ebf 100644
--- a/packages/@uppy/dashboard/lib/components/AddFiles.js
+++ b/packages/@uppy/dashboard/lib/components/AddFiles.js
@@ -2,21 +2,29 @@ import { Component, Fragment, h } from "preact";
 class AddFiles extends Component {
   constructor() {
     super(...arguments);
+    this.fileInput = null;
+    this.folderInput = null;
+    this.mobilePhotoFileInput = null;
+    this.mobileVideoFileInput = null;
     this.triggerFileInputClick = () => {
-      this.fileInput.click();
+      var _this$fileInput;
+      (_this$fileInput = this.fileInput) == null || _this$fileInput.click();
     };
     this.triggerFolderInputClick = () => {
-      this.folderInput.click();
+      var _this$folderInput;
+      (_this$folderInput = this.folderInput) == null || _this$folderInput.click();
     };
     this.triggerVideoCameraInputClick = () => {
-      this.mobileVideoFileInput.click();
+      var _this$mobileVideoFile;
+      (_this$mobileVideoFile = this.mobileVideoFileInput) == null || _this$mobileVideoFile.click();
     };
     this.triggerPhotoCameraInputClick = () => {
-      this.mobilePhotoFileInput.click();
+      var _this$mobilePhotoFile;
+      (_this$mobilePhotoFile = this.mobilePhotoFileInput) == null || _this$mobilePhotoFile.click();
     };
     this.onFileInputChange = event => {
       this.props.handleInputChange(event);
-      event.target.value = null;
+      event.target.value = "";
     };
     this.renderHiddenInput = (isFolder, refCallback) => {
       var _this$props$allowedFi;
@@ -305,16 +313,6 @@ class AddFiles extends Component {
       if (hasOnlyMyDevice) list = [];
       const listWithoutLastTwo = [...list];
       const lastTwo = listWithoutLastTwo.splice(list.length - 2, list.length);
-      const renderList = l =>
-        l.map(_ref => {
-          let {
-            key,
-            elements,
-          } = _ref;
-          return h(Fragment, {
-            key: key,
-          }, elements);
-        });
       return h(
         Fragment,
         null,
@@ -325,13 +323,33 @@ class AddFiles extends Component {
             className: "uppy-Dashboard-AddFiles-list",
             role: "tablist",
           },
-          renderList(listWithoutLastTwo),
-          h("span", {
-            role: "presentation",
-            style: {
-              "white-space": "nowrap",
+          listWithoutLastTwo.map(_ref => {
+            let {
+              key,
+              elements,
+            } = _ref;
+            return h(Fragment, {
+              key: key,
+            }, elements);
+          }),
+          h(
+            "span",
+            {
+              role: "presentation",
+              style: {
+                "white-space": "nowrap",
+              },
             },
-          }, renderList(lastTwo)),
+            lastTwo.map(_ref2 => {
+              let {
+                key,
+                elements,
+              } = _ref2;
+              return h(Fragment, {
+                key: key,
+              }, elements);
+            }),
+          ),
         ),
       );
     };
@@ -413,7 +431,7 @@ class AddFiles extends Component {
         this.props.note && h("div", {
           className: "uppy-Dashboard-note",
         }, this.props.note),
-        this.props.proudlyDisplayPoweredByUppy && this.renderPoweredByUppy(this.props),
+        this.props.proudlyDisplayPoweredByUppy && this.renderPoweredByUppy(),
       ),
     );
   }
diff --git a/packages/@uppy/dashboard/lib/components/Dashboard.js b/packages/@uppy/dashboard/lib/components/Dashboard.js
index 1fb0233..189b65d 100644
--- a/packages/@uppy/dashboard/lib/components/Dashboard.js
+++ b/packages/@uppy/dashboard/lib/components/Dashboard.js
@@ -55,7 +55,7 @@ export default function Dashboard(props) {
   const numberOfFilesForRecovery = props.recoveredState ? Object.keys(props.recoveredState.files).length : null;
   const numberOfGhosts = props.files
     ? Object.keys(props.files).filter(fileID => props.files[fileID].isGhost).length
-    : null;
+    : 0;
   const renderRestoredText = () => {
     if (numberOfGhosts > 0) {
       return props.i18n("recoveredXFiles", {
@@ -191,12 +191,23 @@ export default function Dashboard(props) {
             containerWidth: props.containerWidth,
             containerHeight: props.containerHeight,
           })
-          : h(
-            AddFiles,
-            _extends({}, props, {
-              isSizeMD: isSizeMD,
-            }),
-          ),
+          : h(AddFiles, {
+            i18n: props.i18n,
+            i18nArray: props.i18nArray,
+            acquirers: props.acquirers,
+            handleInputChange: props.handleInputChange,
+            maxNumberOfFiles: props.maxNumberOfFiles,
+            allowedFileTypes: props.allowedFileTypes,
+            showNativePhotoCameraButton: props.showNativePhotoCameraButton,
+            showNativeVideoCameraButton: props.showNativeVideoCameraButton,
+            nativeCameraFacingMode: props.nativeCameraFacingMode,
+            showPanel: props.showPanel,
+            activePickerPanel: props.activePickerPanel,
+            disableLocalFiles: props.disableLocalFiles,
+            fileManagerSelectionType: props.fileManagerSelectionType,
+            note: props.note,
+            proudlyDisplayPoweredByUppy: props.proudlyDisplayPoweredByUppy,
+          }),
         h(
           Slide,
           null,

[aduh95]

Suggested change

	folderInput: HTMLInputElement | null = null
	folderInput!: HTMLInputElement

And let's remove the optional chaining below

[lakesare]

🤔 why
I understand ref is unlilkely to be null when triggerFileInputClick() is called, but it is certainly null by default. So, if we want to say "we know this is not null now", we should instead do this.fileInput!.click().

But I wouldn't do that either, I think we shouldn't rely on react rendering cycles too much.

[aduh95]

To reduce the output diff

[lakesare]

I mean - reducing the output diff is clearly not the value in itself, and we certainly shouldn't ! values in order to reduce the output diff.

I think your logic is "when I'm viewing the git history of the file, I should be able to ignore typescript commits, because they are highly unlikely to introduce real changes"?
If so - would you like me to subdivide this PR into some chunks, especially for changes that do change output js?

[aduh95]

I guess there are several points for which it makes sense to me to keep the output diff as close to 0:

• I believe there is value in not artificially increasing the output files size when (IMO in this case) it does not bring any value to our users, and very little to ourselves.
• It makes reviewing so much easier.
• As you said, it makes git blame crawling easier because the TS commit can be safely discarded.

Now I don't feel strongly about either of those points, feel free to ignore, but if you think it does make sense, I'm happy to help split the PR so we can land the uncontroversial TS-only changes first.

[lakesare]

I believe there is value in not artificially increasing the output files size when (IMO in this case) it does not bring any value to our users, and very little to ourselves.

I think we should certainly defend against null refs here - file size increase is a negligible cost for safety and a sense of "this will certainly not error out, no matter the react internal life cycles" for developers. To be clear - file size increase is not always a negligible cost, but this file size increase is certainly a negligible cost.

I'm pro doing:

• ;(event.target as HTMLInputElement).value = '' as a separate PR
• accept={this.props.allowedFileTypes?.join(', ')} as a separate PR

Because this is actual functionality change, and it's useful to create PRs that are focused on one change specifically (more motivation to actually test it in multiple browsers for both the author and reviewers; better git blames).

I'll PR those separately 👍

[lakesare]

Two PRs:

• .handleInputChange() - use .currentTarget; clear the input using '' #5381
• Transform the accept prop into a string everywhere #5380

PRing these separately turned out to be a good idea, we needed to change these not only in Dashboard, but in several other plugins too.

[aduh95]

Let's try to reduce the changes in #5367 (comment) as close to 0 as possible

[lakesare]

@aduh95, as far as I see the only change that I can remove there is to return the renderList function.

I don't believe it's difficult to review here - if you think the version with the renderList was an easier read, I can revert it.

[aduh95]

@lakesare also the changes in AddFiles. If we have a PR with passing tests and no output diff, the reviewing is very straight forward, and it can land almost immediately. The changes that require changing the output/distributed files require more scrutiny.

[lakesare]

@lakesare also the changes in AddFiles. If we have a PR with passing tests and no output diff, the reviewing is very straight forward, and it can land almost immediately. The changes that require changing the output/distributed files require more scrutiny.

We don't need to land it immediately, these are genuine changes! Please review it as if the errors can crawl in, and then I'll throw my eye over it again before merging.
I understand the value of this diff when we're converting files that do not necessitate the change to functionality, but dashboard was relatively a mess type-wise, we did need to change stuff there.

[mifi]

This pr needs to be altered slightly after #5381 is merged, but other than that LGTM. Improving our flawed typings and readability outweighs bigger git diffs and a slight bundle size increase imo.

in the future we can probably increase our target browser supported versions so we don't have to transpile things like this:

-      this.folderInput?.click();
+      var _this$folderInput;
+      (_this$folderInput = this.folderInput) == null || _this$folderInput.click();