Support multiple messages in informer (#3017)

* Support multiple messages in informer

* Fix informer duplicate file messages for folders

Co-authored-by: Artur Paikin <artur@arturpaikin.com>

packages/@uppy/core/src/getFileName.js

@@ -0,0 +1,11 @@
+module.exports = function getFileName (fileType, fileDescriptor) {
+  if (fileDescriptor.name) {
+    return fileDescriptor.name
+  }
+
+  if (fileType.split('/')[0] === 'image') {
+    return `${fileType.split('/')[0]}.${fileType.split('/')[1]}`
+  }
+
+  return 'noname'
+}

packages/@uppy/core/src/index.js

@@ -10,6 +10,7 @@ const getFileType = require('@uppy/utils/lib/getFileType')
 const getFileNameAndExtension = require('@uppy/utils/lib/getFileNameAndExtension')
 const generateFileID = require('@uppy/utils/lib/generateFileID')
 const supportsUploadProgress = require('./supportsUploadProgress')
+const getFileName = require('./getFileName')
 const { justErrorsLogger, debugLogger } = require('./loggers')
 const UIPlugin = require('./UIPlugin')
 const BasePlugin = require('./BasePlugin')
@@ -95,6 +96,7 @@ class Uppy {
         enterTextToSearch: 'Enter text to search for images',
         backToSearch: 'Back to Search',
         emptyFolderAdded: 'No files were added from empty folder',
+        folderAlreadyAdded: 'The folder "%{folder}" was already added',
         folderAdded: {
           0: 'Added %{smart_count} file from %{folder}',
           1: 'Added %{smart_count} files from %{folder}',
@@ -197,11 +199,7 @@ class Uppy {
       },
       totalProgress: 0,
       meta: { ...this.opts.meta },
-      info: {
-        isHidden: true,
-        type: 'info',
-        message: '',
-      },
+      info: [],
       recoveredState: null,
     })
 
@@ -613,25 +611,26 @@ class Uppy {
     }
   }
 
+  checkIfFileAlreadyExists (fileID) {
+    const { files } = this.getState()
+
+    if (files[fileID] && !files[fileID].isGhost) {
+      return true
+    }
+    return false
+  }
+
   /**
    * Create a file state object based on user-provided `addFile()` options.
    *
-   * Note this is extremely side-effectful and should only be done when a file state object will be added to state
-   * immediately afterward!
+   * Note this is extremely side-effectful and should only be done when a file state object
+   * will be added to state immediately afterward!
    *
    * The `files` value is passed in because it may be updated by the caller without updating the store.
    */
   #checkAndCreateFileStateObject (files, fileDescriptor) {
     const fileType = getFileType(fileDescriptor)
-
-    let fileName
-    if (fileDescriptor.name) {
-      fileName = fileDescriptor.name
-    } else if (fileType.split('/')[0] === 'image') {
-      fileName = `${fileType.split('/')[0]}.${fileType.split('/')[1]}`
-    } else {
-      fileName = 'noname'
-    }
+    const fileName = getFileName(fileType, fileDescriptor)
     const fileExtension = getFileNameAndExtension(fileName).extension
     const isRemote = Boolean(fileDescriptor.isRemote)
     const fileID = generateFileID({
@@ -639,11 +638,9 @@ class Uppy {
       type: fileType,
     })
 
-    if (files[fileID] && !files[fileID].isGhost) {
-      this.#showOrLogErrorAndThrow(
-        new RestrictionError(this.i18n('noDuplicates', { fileName })),
-        { fileDescriptor }
-      )
+    if (this.checkIfFileAlreadyExists(fileID)) {
+      const error = new RestrictionError(this.i18n('noDuplicates', { fileName }))
+      this.showOrLogErrorAndThrow(error, { file: fileDescriptor })
     }
 
     const meta = fileDescriptor.meta || {}
@@ -1429,31 +1426,26 @@ class Uppy {
     const isComplexMessage = typeof message === 'object'
 
     this.setState({
-      info: {
-        isHidden: false,
-        type,
-        message: isComplexMessage ? message.message : message,
-        details: isComplexMessage ? message.details : null,
-      },
+      info: [
+        ...this.getState().info,
+        {
+          type,
+          message: isComplexMessage ? message.message : message,
+          details: isComplexMessage ? message.details : null,
+        },
+      ],
     })
 
-    this.emit('info-visible')
-
-    clearTimeout(this.infoTimeoutID)
-    if (duration === 0) {
-      this.infoTimeoutID = undefined
-      return
-    }
+    setTimeout(this.hideInfo, duration)
 
-    // hide the informer after `duration` milliseconds
-    this.infoTimeoutID = setTimeout(this.hideInfo, duration)
+    this.emit('info-visible')
   }
 
   hideInfo () {
-    const newInfo = { ...this.getState().info, isHidden: true }
-    this.setState({
-      info: newInfo,
-    })
+    const { info } = this.getState()
+
+    this.setState({ info: info.slice(1) })
+
     this.emit('info-hidden')
   }
 

packages/@uppy/core/src/index.test.js

@@ -153,7 +153,7 @@ describe('src/Core', () => {
         currentUploads: {},
         allowNewUpload: true,
         foo: 'baar',
-        info: { isHidden: true, message: '', type: 'info' },
+        info: [],
         meta: {},
         plugins: {},
         totalProgress: 0,
@@ -178,7 +178,7 @@ describe('src/Core', () => {
         currentUploads: {},
         allowNewUpload: true,
         foo: 'bar',
-        info: { isHidden: true, message: '', type: 'info' },
+        info: [],
         meta: {},
         plugins: {},
         totalProgress: 0,
@@ -192,7 +192,7 @@ describe('src/Core', () => {
         currentUploads: {},
         allowNewUpload: true,
         foo: 'baar',
-        info: { isHidden: true, message: '', type: 'info' },
+        info: [],
         meta: {},
         plugins: {},
         totalProgress: 0,
@@ -232,7 +232,7 @@ describe('src/Core', () => {
       allowNewUpload: true,
       error: null,
       foo: 'bar',
-      info: { isHidden: true, message: '', type: 'info' },
+      info: [],
       meta: {},
       plugins: {},
       totalProgress: 0,
@@ -293,7 +293,7 @@ describe('src/Core', () => {
       currentUploads: {},
       allowNewUpload: true,
       error: null,
-      info: { isHidden: true, message: '', type: 'info' },
+      info: [],
       meta: {},
       plugins: {},
       totalProgress: 0,
@@ -329,16 +329,16 @@ describe('src/Core', () => {
   describe('preprocessors', () => {
     it('should add a preprocessor', () => {
       const core = new Core()
-      const preprocessor = () => {}
+      const preprocessor = () => { }
       core.addPreProcessor(preprocessor)
       expect(core.preProcessors[0]).toEqual(preprocessor)
     })
 
     it('should remove a preprocessor', () => {
       const core = new Core()
-      const preprocessor1 = () => {}
-      const preprocessor2 = () => {}
-      const preprocessor3 = () => {}
+      const preprocessor1 = () => { }
+      const preprocessor2 = () => { }
+      const preprocessor3 = () => { }
       core.addPreProcessor(preprocessor1)
       core.addPreProcessor(preprocessor2)
       core.addPreProcessor(preprocessor3)
@@ -458,16 +458,16 @@ describe('src/Core', () => {
   describe('postprocessors', () => {
     it('should add a postprocessor', () => {
       const core = new Core()
-      const postprocessor = () => {}
+      const postprocessor = () => { }
       core.addPostProcessor(postprocessor)
       expect(core.postProcessors[0]).toEqual(postprocessor)
     })
 
     it('should remove a postprocessor', () => {
       const core = new Core()
-      const postprocessor1 = () => {}
-      const postprocessor2 = () => {}
-      const postprocessor3 = () => {}
+      const postprocessor1 = () => { }
+      const postprocessor2 = () => { }
+      const postprocessor3 = () => { }
       core.addPostProcessor(postprocessor1)
       core.addPostProcessor(postprocessor2)
       core.addPostProcessor(postprocessor3)
@@ -588,16 +588,16 @@ describe('src/Core', () => {
   describe('uploaders', () => {
     it('should add an uploader', () => {
       const core = new Core()
-      const uploader = () => {}
+      const uploader = () => { }
       core.addUploader(uploader)
       expect(core.uploaders[0]).toEqual(uploader)
     })
 
     it('should remove an uploader', () => {
       const core = new Core()
-      const uploader1 = () => {}
-      const uploader2 = () => {}
-      const uploader3 = () => {}
+      const uploader1 = () => { }
+      const uploader2 = () => { }
+      const uploader3 = () => { }
       core.addUploader(uploader1)
       core.addUploader(uploader2)
       core.addUploader(uploader3)
@@ -1065,9 +1065,9 @@ describe('src/Core', () => {
   })
 
   describe('restoring a file', () => {
-    xit('should restore a file', () => {})
+    xit('should restore a file', () => { })
 
-    xit("should fail to restore a file if it doesn't exist", () => {})
+    xit("should fail to restore a file if it doesn't exist", () => { })
   })
 
   describe('get a file', () => {
@@ -1538,11 +1538,11 @@ describe('src/Core', () => {
         throw new Error('should have thrown')
       } catch (err) {
         expect(err).toMatchObject(new Error('You can only upload 1 file'))
-        expect(core.getState().info.message).toEqual('You can only upload 1 file')
+        expect(core.getState().info[0].message).toEqual('You can only upload 1 file')
       }
     })
 
-    xit('should enforce the minNumberOfFiles rule', () => {})
+    xit('should enforce the minNumberOfFiles rule', () => { })
 
     it('should enforce the allowedFileTypes rule', () => {
       const core = new Core({
@@ -1561,7 +1561,7 @@ describe('src/Core', () => {
         throw new Error('should have thrown')
       } catch (err) {
         expect(err).toMatchObject(new Error('You can only upload: image/gif, image/png'))
-        expect(core.getState().info.message).toEqual('You can only upload: image/gif, image/png')
+        expect(core.getState().info[0].message).toEqual('You can only upload: image/gif, image/png')
       }
     })
 
@@ -1595,7 +1595,7 @@ describe('src/Core', () => {
         throw new Error('should have thrown')
       } catch (err) {
         expect(err).toMatchObject(new Error('You can only upload: .gif, .jpg, .jpeg'))
-        expect(core.getState().info.message).toEqual('You can only upload: .gif, .jpg, .jpeg')
+        expect(core.getState().info[0].message).toEqual('You can only upload: .gif, .jpg, .jpeg')
       }
 
       expect(() => core.addFile({
@@ -1623,7 +1623,7 @@ describe('src/Core', () => {
         throw new Error('should have thrown')
       } catch (err) {
         expect(err).toMatchObject(new Error('foo.jpg exceeds maximum allowed size of 1.2 KB'))
-        expect(core.getState().info.message).toEqual('foo.jpg exceeds maximum allowed size of 1.2 KB')
+        expect(core.getState().info[0].message).toEqual('foo.jpg exceeds maximum allowed size of 1.2 KB')
       }
     })
 
@@ -1644,7 +1644,7 @@ describe('src/Core', () => {
         throw new Error('should have thrown')
       } catch (err) {
         expect(err).toMatchObject(new Error('This file is smaller than the allowed size of 1 GB'))
-        expect(core.getState().info.message).toEqual('This file is smaller than the allowed size of 1 GB')
+        expect(core.getState().info[0].message).toEqual('This file is smaller than the allowed size of 1 GB')
       }
     })
 
@@ -1760,12 +1760,11 @@ describe('src/Core', () => {
         },
       })
       core.emit('upload-error', core.getFile('fileId'), new Error('this is the error'))
-      expect(core.getState().info).toEqual({
+      expect(core.getState().info).toEqual([{
         message: 'Failed to upload filename',
         details: 'this is the error',
-        isHidden: false,
         type: 'error',
-      })
+      }])
     })
 
     it('should reset the error state when receiving the upload event', () => {
@@ -1830,14 +1829,12 @@ describe('src/Core', () => {
       core.on('info-visible', infoVisibleEvent)
 
       core.info('This is the message', 'info', 0)
-      expect(core.getState().info).toEqual({
-        isHidden: false,
+      expect(core.getState().info).toEqual([{
         type: 'info',
         message: 'This is the message',
         details: null,
-      })
+      }])
       expect(infoVisibleEvent.mock.calls.length).toEqual(1)
-      expect(typeof core.infoTimeoutID).toEqual('undefined')
     })
 
     it('should set a object based message to be displayed infinitely', () => {
@@ -1851,16 +1848,14 @@ describe('src/Core', () => {
           foo: 'bar',
         },
       }, 'warning', 0)
-      expect(core.getState().info).toEqual({
-        isHidden: false,
+      expect(core.getState().info).toEqual([{
         type: 'warning',
         message: 'This is the message',
         details: {
           foo: 'bar',
         },
-      })
+      }])
       expect(infoVisibleEvent.mock.calls.length).toEqual(1)
-      expect(typeof core.infoTimeoutID).toEqual('undefined')
     })
 
     it('should set an info message to be displayed for a period of time before hiding', (done) => {
@@ -1871,16 +1866,10 @@ describe('src/Core', () => {
       core.on('info-hidden', infoHiddenEvent)
 
       core.info('This is the message', 'info', 100)
-      expect(typeof core.infoTimeoutID).toEqual('number')
       expect(infoHiddenEvent.mock.calls.length).toEqual(0)
       setTimeout(() => {
         expect(infoHiddenEvent.mock.calls.length).toEqual(1)
-        expect(core.getState().info).toEqual({
-          isHidden: true,
-          type: 'info',
-          message: 'This is the message',
-          details: null,
-        })
+        expect(core.getState().info).toEqual([])
         done()
       }, 110)
     })
@@ -1893,16 +1882,50 @@ describe('src/Core', () => {
       core.on('info-hidden', infoHiddenEvent)
 
       core.info('This is the message', 'info', 0)
-      expect(typeof core.infoTimeoutID).toEqual('undefined')
       expect(infoHiddenEvent.mock.calls.length).toEqual(0)
       core.hideInfo()
       expect(infoHiddenEvent.mock.calls.length).toEqual(1)
-      expect(core.getState().info).toEqual({
-        isHidden: true,
-        type: 'info',
-        message: 'This is the message',
-        details: null,
-      })
+      expect(core.getState().info).toEqual([])
+    })
+
+    it('should support multiple messages', () => {
+      const infoVisibleEvent = jest.fn()
+      const infoHiddenEvent = jest.fn()
+      const core = new Core()
+
+      core.on('info-visible', infoVisibleEvent)
+      core.on('info-hidden', infoHiddenEvent)
+
+      core.info('This is the message', 'info', 0)
+      core.info('But this is another one', 'info', 0)
+
+      expect(infoHiddenEvent.mock.calls.length).toEqual(0)
+      expect(core.getState().info).toEqual([
+        {
+          type: 'info',
+          message: 'This is the message',
+          details: null,
+        },
+        {
+          type: 'info',
+          message: 'But this is another one',
+          details: null,
+        },
+      ])
+      core.hideInfo()
+
+      expect(core.getState().info).toEqual([
+        {
+          type: 'info',
+          message: 'But this is another one',
+          details: null,
+        },
+      ])
+
+      core.hideInfo()
+
+      expect(infoHiddenEvent.mock.calls.length).toEqual(2)
+      expect(core.getState().info).toEqual([])
     })
   })
 

packages/@uppy/informer/package.json

@@ -24,7 +24,8 @@
   },
   "dependencies": {
     "@uppy/utils": "file:../utils",
-    "preact": "^10.5.13"
+    "preact": "^10.5.13",
+    "preact-transition-group": "^2.0.0"
   },
   "peerDependencies": {
     "@uppy/core": "^1.0.0"

packages/@uppy/informer/src/FadeIn.js

@@ -0,0 +1,29 @@
+const { h, Component, createRef } = require('preact')
+
+const TRANSITION_MS = 300
+
+module.exports = class FadeIn extends Component {
+  ref = createRef();
+
+  componentWillEnter (callback) {
+    this.ref.current.style.opacity = '1'
+    this.ref.current.style.transform = 'none'
+    setTimeout(callback, TRANSITION_MS)
+  }
+
+  componentWillLeave (callback) {
+    this.ref.current.style.opacity = '0'
+    this.ref.current.style.transform = 'translateY(350%)'
+    setTimeout(callback, TRANSITION_MS)
+  }
+
+  render () {
+    const { children } = this.props
+
+    return (
+      <div className="uppy-Informer-animated" ref={this.ref}>
+        {children}
+      </div>
+    )
+  }
+}

packages/@uppy/informer/src/index.js

@@ -1,5 +1,9 @@
-const { UIPlugin } = require('@uppy/core')
+/* eslint-disable jsx-a11y/no-noninteractive-element-interactions  */
+/* eslint-disable jsx-a11y/click-events-have-key-events */
 const { h } = require('preact')
+const TransitionGroup = require('preact-transition-group')
+const { UIPlugin } = require('@uppy/core')
+const FadeIn = require('./FadeIn')
 
 /**
  * Informer
@@ -9,6 +13,7 @@ const { h } = require('preact')
  *
  */
 module.exports = class Informer extends UIPlugin {
+  // eslint-disable-next-line global-require
   static VERSION = require('../package.json').version
 
   constructor (uppy, opts) {
@@ -24,43 +29,30 @@ module.exports = class Informer extends UIPlugin {
   }
 
   render = (state) => {
-    const { isHidden, message, details } = state.info
-
-    function displayErrorAlert () {
-      const errorMessage = `${message} \n\n ${details}`
-      alert(errorMessage)
-    }
-
-    const handleMouseOver = () => {
-      clearTimeout(this.uppy.infoTimeoutID)
-    }
-
-    const handleMouseLeave = () => {
-      this.uppy.infoTimeoutID = setTimeout(this.uppy.hideInfo, 2000)
-    }
-
     return (
-      <div
-        className="uppy uppy-Informer"
-        aria-hidden={isHidden}
-      >
-        <p role="alert">
-          {message}
-          {' '}
-          {details && (
-            <span
-              aria-label={details}
-              data-microtip-position="top-left"
-              data-microtip-size="medium"
-              role="tooltip"
-              onClick={displayErrorAlert}
-              onMouseOver={handleMouseOver}
-              onMouseLeave={handleMouseLeave}
-            >
-              ?
-            </span>
-          )}
-        </p>
+      <div className="uppy uppy-Informer">
+        <TransitionGroup>
+          {state.info.map((info) => (
+            <FadeIn key={info.message}>
+              <p role="alert">
+                {info.message}
+                {' '}
+                {info.details && (
+                  <span
+                    aria-label={info.details}
+                    data-microtip-position="top-left"
+                    data-microtip-size="medium"
+                    role="tooltip"
+                    // eslint-disable-next-line no-alert
+                    onClick={() => alert(`${info.message} \n\n ${info.details}`)}
+                  >
+                    ?
+                  </span>
+                )}
+              </p>
+            </FadeIn>
+          ))}
+        </TransitionGroup>
       </div>
     )
   }

packages/@uppy/informer/src/style.scss

@@ -7,18 +7,19 @@
   left: 0;
   right: 0;
   text-align: center;
-  opacity: 1;
-  transform: none;
-  transition: all 250ms ease-in;
   z-index: $zIndex-5;
+   
+  span > div {
+    margin-bottom: 6px;
+  }
 }
 
-  .uppy-Informer[aria-hidden=true] {
-    opacity: 0;
-    transform: translateY(350%);
-    transition: all 300ms ease-in;
-    z-index: $zIndex-negative;
-  }
+.uppy-Informer-animated {
+  opacity: 0;
+  transform: translateY(350%);
+  transition: all 300ms ease-in;
+  z-index: $zIndex-negative;
+}
 
 .uppy-Informer p {
   display: inline-block;
@@ -45,7 +46,7 @@
   }
 }
 
-.uppy-Informer span {
+.uppy-Informer p span {
   line-height: 12px;
   width: 13px;
   height: 13px;
@@ -61,11 +62,11 @@
   margin-inline-start: -1px;
 }
 
-  .uppy-Informer span:hover {
+  .uppy-Informer p span:hover {
     cursor: help;
   }
 
-  .uppy-Informer span:after {
+  .uppy-Informer p span:after {
     line-height: 1.3;
     word-wrap: break-word;
   }

packages/@uppy/provider-views/src/ProviderView/ProviderView.js

@@ -304,18 +304,36 @@ module.exports = class ProviderView {
     const folderId = this.providerFileToId(folder)
     const state = this.plugin.getPluginState()
     const folders = { ...state.selectedFolders }
+
     if (folderId in folders && folders[folderId].loading) {
       return
     }
+
     folders[folderId] = { loading: true, files: [] }
+
     this.plugin.setPluginState({ selectedFolders: { ...folders } })
+
+    // eslint-disable-next-line consistent-return
     return this.listAllFiles(folder.requestPath).then((files) => {
       let count = 0
-      files.forEach((file) => {
-        const success = this.addFile(file)
-        if (success) count++
+
+      // If the same folder is added again, we don't want to send
+      // X amount of duplicate file notifications, we want to say
+      // the folder was already added. This checks if all files are duplicate,
+      // if that's the case, we don't add the files.
+      files.forEach(file => {
+        const id = this.providerFileToId(file)
+        if (!this.plugin.uppy.checkIfFileAlreadyExists(id)) {
+          count++
+        }
       })
+
+      if (count > 0) {
+        files.forEach((file) => this.addFile(file))
+      }
+
       const ids = files.map(this.providerFileToId)
+
       folders[folderId] = {
         loading: false,
         files: ids,
@@ -323,13 +341,19 @@ module.exports = class ProviderView {
       this.plugin.setPluginState({ selectedFolders: folders })
 
       let message
-      if (files.length) {
+
+      if (count === 0) {
+        message = this.plugin.uppy.i18n('folderAlreadyAdded', {
+          folder: folder.name,
+        })
+      } else if (files.length) {
         message = this.plugin.uppy.i18n('folderAdded', {
           smart_count: count, folder: folder.name,
         })
       } else {
         message = this.plugin.uppy.i18n('emptyFolderAdded')
       }
+
       this.plugin.uppy.info(message)
     }).catch((e) => {
       const state = this.plugin.getPluginState()