fix(GUI): file-picker performance and design improvements

- Replace onClick arrow functions in all components that use them for
  efficiency reasons: 300-500% speed-up
- Sort by folders and ignore case for better UX
- Remove use of `rendition.Button` in files, leading to a 10-20%
  performance increase when browsing files
- Proper sidebar width and spacing
- Recents and favorites are now filtered by existence async for a tiny
  performance improvement
- Make Breadcrumbs and Icon pure components to stop frequent re-rendering
- Initial support for array constraints
- Use first constraint as initial path instead of homedir if a
  constraint is set
- Use correct design height on modal, `calc(100vh - 20px)`
- Reset scroll position when browsing a new folder
- Fuse Bluebird `.map()` and `.reduce()` in
  `files.getAllFilesMetadataAsync`.
- Use `localeCompare`'s own case-insensitive option instead of calling
  `.toLowerCase()` twice on `n-2` files compared.
- Use 16px font sizes in sidebar and files to match design.
- Disable `$locationProvider.html5Mode.rewriteLinks`, which seemed to
  take 50ms of the directory changing time.
- Leave file extension as-is in `files.getFileMetadataSync` and the
  async counterpart for a very minor performance improvement.

Change-Type: patch

lib/gui/app/app.js

@@ -330,6 +330,13 @@ app.config(($provide) => {
   })
 })
 
+app.config(($locationProvider) => {
+  // NOTE(Shou): this seems to invoke a minor perf decrease when set to true
+  $locationProvider.html5Mode({
+    rewriteLinks: false
+  })
+})
+
 app.controller('HeaderController', function (OSOpenExternalService) {
   /**
    * @summary Open help page

lib/gui/app/components/file-selector/controllers/file-selector.js

@@ -16,8 +16,11 @@
 
 'use strict'
 
+const os = require('os')
 const settings = require('../../../models/settings')
 
+/* eslint-disable lodash/prefer-lodash-method */
+
 module.exports = function (
   $uibModalInstance
 ) {
@@ -43,9 +46,25 @@ module.exports = function (
    * @example
    * FileSelectorController.getFolderConstraint()
    */
-  this.getFolderConstraint = () => {
+  this.getFolderConstraints = () => {
     // TODO(Shou): get this dynamically from the mountpoint of a specific port in Etcher Pro
-    // TODO: Make this handle multiple constraints
+    return settings.has('fileBrowserConstraintPath')
-    return settings.get('fileBrowserConstraintPath')
+      ? settings.get('fileBrowserConstraintPath').split(',')
+      : []
+  }
+
+  /**
+   * @summary Get initial path
+   * @function
+   * @public
+   *
+   * @returns {String} - path
+   *
+   * @example
+   * <file-selector path="FileSelectorController.getPath()"></file-selector>
+   */
+  this.getPath = () => {
+    const [ constraint ] = this.getFolderConstraints()
+    return constraint || os.homedir()
   }
 }

lib/gui/app/components/file-selector/file-selector/file-selector.jsx

@@ -17,7 +17,6 @@
 'use strict'
 
 const _ = require('lodash')
-const os = require('os')
 const fs = require('fs')
 const path = require('path')
 const React = require('react')
@@ -36,7 +35,7 @@ const {
 const Storage = require('../../../models/storage')
 const analytics = require('../../../modules/analytics')
 const middleEllipsis = require('../../../utils/middle-ellipsis')
-const files = require('../../../../../shared/files')
+const files = require('../../../models/files')
 const selectionState = require('../../../models/selection-state')
 const imageStream = require('../../../../../sdk/image-stream')
 const errors = require('../../../../../shared/errors')
@@ -92,6 +91,9 @@ const colors = {
   highlight: {
     color: 'white',
     background: '#2297de'
+  },
+  soft: {
+    color: '#4d5056'
   }
 }
 
@@ -109,16 +111,25 @@ const icons = {
 
 /**
  * @summary Icon React component
- * @function
+ * @class
  * @type {ReactElement}
  */
-const Icon = styled((props) => {
+class UnstyledIcon extends React.PureComponent {
-  const { type, ...restProps } = props
+  render () {
+    const { type, ...restProps } = this.props
 
-  return (
+    return (
-    <span className={ props.className } dangerouslySetInnerHTML={ { __html: icons[type] } } { ...restProps } />
+      <span className={ this.props.className } dangerouslySetInnerHTML={ { __html: icons[type] } } { ...restProps } />
-  )
+    )
-})`
+  }
+}
+
+/**
+ * @summary Icon Styled component
+ * @function
+ * @type {StyledComponent}
+ */
+const Icon = styled(UnstyledIcon)`
   color: ${props => props.color};
   font-size: ${props => props.size};
 `
@@ -138,25 +149,44 @@ const Flex = styled.div`
   flex-grow: ${ props => props.grow };
 `
 
-const FileLink = styled((props) => {
+class UnstyledFileLink extends React.PureComponent {
-  const icon = props.isDirectory ? 'faFolder' : 'faFileAlt'
+  constructor (props) {
+    super(props)
 
-  return (
+    this.highlightFile = this.highlightFile.bind(this)
-    <Flex
+    this.selectFile = this.selectFile.bind(this)
-      direction="column"
+  }
-      alignItems="stretch"
+
-      className={ props.className }
+  render () {
-      onClick={ props.onClick }
+    const icon = this.props.file.isDirectory ? 'faFolder' : 'faFileAlt'
-      onDoubleClick={ props.onDoubleClick }>
+
-      <Icon type={ icon } size="48px" />
+    return (
-      <rendition.Button plaintext={ true }>
+      <Flex
-        { middleEllipsis(props.basename || '', FILENAME_CHAR_LIMIT) }
+        direction="column"
-      </rendition.Button>
+        alignItems="stretch"
-      <div>{ prettyBytes(props.size || 0) }</div>
+        className={ this.props.className }
-    </Flex>
+        onClick={ this.highlightFile }
-  )
+        onDoubleClick={ this.selectFile }>
-})`
+        <Icon type={ icon } size="48px" />
-  width: 80px;
+        <span>
+          { middleEllipsis(this.props.file.basename || '', FILENAME_CHAR_LIMIT_SHORT) }
+        </span>
+        <div>{ prettyBytes(this.props.file.size || 0) }</div>
+      </Flex>
+    )
+  }
+
+  highlightFile () {
+    this.props.highlightFile(this.props.file)
+  }
+
+  selectFile () {
+    this.props.selectFile(this.props.file)
+  }
+}
+
+const FileLink = styled(UnstyledFileLink)`
+  width: 100px;
   max-height: 128px;
   margin: 5px 10px;
   padding: 5px;
@@ -166,17 +196,19 @@ const FileLink = styled((props) => {
   cursor: pointer;
   border-radius: 5px;
 
-  > span:first-child {
+  > span:first-of-type {
     align-self: center;
     line-height: 1;
     margin-bottom: 6px;
+    color: ${ props => props.highlight ? colors.highlight.color : colors.soft.color }
   }
 
-  > button,
+  > span:last-of-type {
-  > button:hover,
+    display: flex;
-  > button:focus {
+    justify-content: center;
-    color: inherit;
+    text-align: center;
     word-break: break-all;
+    font-size: 16px;
   }
 
   > div:last-child {
@@ -206,12 +238,66 @@ const Footer = Flex.extend`
   }
 `
 
-const FileListWrap = Flex.extend`
+class UnstyledFileListWrap extends React.PureComponent {
+  constructor (props) {
+    super(props)
+
+    this.scrollElem = null
+
+    this.setScrollElem = this.setScrollElem.bind(this)
+  }
+
+  render () {
+    return (
+      <Flex
+        className={ this.props.className }
+        innerRef={ this.setScrollElem }
+        wrap="wrap"
+      >
+        { this.props.children }
+      </Flex>
+    )
+  }
+
+  setScrollElem (scrollElem) {
+    this.scrollElem = scrollElem
+  }
+
+  componentDidUpdate (prevProps) {
+    if (prevProps.path !== this.props.path && this.scrollElem) {
+      this.scrollElem.scrollTop = 0
+    }
+  }
+}
+
+const FileListWrap = styled(UnstyledFileListWrap)`
   overflow-x: hidden;
   overflow-y: auto;
   padding: 0 20px;
 `
 
+class RecentFileLink extends React.PureComponent {
+  constructor (props) {
+    super(props)
+
+    this.select = this.select.bind(this)
+  }
+
+  render () {
+    return (
+      <rendition.Button
+        onClick={ this.select }
+        plaintext={ true }>
+        { middleEllipsis(path.basename(this.props.title), FILENAME_CHAR_LIMIT_SHORT) }
+      </rendition.Button>
+    )
+  }
+
+  select () {
+    files.getFileMetadataAsync(this.props.fullpath).then(this.props.selectFile)
+  }
+}
+
 class RecentFilesUnstyled extends React.PureComponent {
   constructor (props) {
     super(props)
@@ -223,37 +309,31 @@ class RecentFilesUnstyled extends React.PureComponent {
   }
 
   render () {
-    const existing = (fileObjs) => {
-      return _.filter(fileObjs, (fileObj) => {
-        return fs.existsSync(fileObj.fullpath)
-      })
-    }
-
     return (
       <Flex className={ this.props.className }>
         <h5>Recent</h5>
         {
-          _.map(existing(this.state.recents), (file) => {
+          _.map(this.state.recents, (file) => {
             return (
-              <rendition.Button
+              <RecentFileLink
-                key={ file.fullpath }
+                key={ file.dirname }
-                onClick={ () => this.props.selectFile(files.getFileMetadataSync(file.dirname)) }
+                fullpath={ file.dirname }
-                plaintext={ true }>
+                title={ path.basename(file.dirname) }
-                { middleEllipsis(path.basename(file.dirname), FILENAME_CHAR_LIMIT_SHORT) }
+                selectFile={ this.props.selectFile }
-              </rendition.Button>
+              />
             )
           })
         }
         <h5>Favorite</h5>
         {
-          _.map(existing(this.state.favorites.slice(0, 4)), (file) => {
+          _.map(this.state.favorites.slice(0, 4), (file) => {
             return (
-              <rendition.Button
+              <RecentFileLink
                 key={ file.fullpath }
-                onClick={ () => this.props.selectFile(files.getFileMetadataSync(file.fullpath)) }
+                fullpath={ file.fullpath }
-                plaintext={ true }>
+                title={ file.basename }
-                { middleEllipsis(file.basename, FILENAME_CHAR_LIMIT_SHORT) }
+                selectFile={ this.props.selectFile }
-              </rendition.Button>
+              />
             )
           })
         }
@@ -269,6 +349,32 @@ class RecentFilesUnstyled extends React.PureComponent {
     window.removeEventListener('storage', this.onStorage)
   }
 
+  componentDidMount () {
+    Bluebird.reduce(this.state.recents, (newRecents, recent) => {
+      return files.existsAsync(recent.fullpath).then((exists) => {
+        if (exists) {
+          return newRecents.concat(recent)
+        }
+
+        return newRecents
+      })
+    }, []).then((recents) => {
+      this.setState({ recents })
+    })
+
+    Bluebird.reduce(this.state.favorites, (newFavorites, favorite) => {
+      return files.existsAsync(favorite.fullpath).then((exists) => {
+        if (exists) {
+          return newFavorites.concat(favorite)
+        }
+
+        return newFavorites
+      })
+    }, []).then((favorites) => {
+      this.setState({ favorites })
+    })
+  }
+
   onStorage (event) {
     if (event.key === RECENT_FILES_KEY) {
       this.setState(event.newValue)
@@ -278,6 +384,7 @@ class RecentFilesUnstyled extends React.PureComponent {
 
 const RecentFiles = styled(RecentFilesUnstyled)`
   display: flex;
+  flex: 0 0 auto;
   flex-direction: column;
   align-items: flex-start;
   width: 130px;
@@ -293,43 +400,80 @@ const RecentFiles = styled(RecentFilesUnstyled)`
     margin-bottom: 15px;
   }
 
+  > h5:last-of-type {
+    margin-top: 20px;
+  }
+
   > button {
     margin-bottom: 10px;
     text-align: start;
+    font-size: 16px;
   }
 `
 
 const labels = {
-  '/': 'Root'
+  '/': 'Root',
+  'mountpoints': 'Mountpoints'
 }
 
-const Breadcrumbs = styled((props) => {
+class Crumb extends React.PureComponent {
-  const folderConstraint = props.constraint || path.parse(props.path).root
+  constructor (props) {
-  const dirs = files.subpaths(props.path).filter((subpath) => {
+    super(props)
-    // Guard against displaying folders outside the constrained folder
-    return !path.relative(folderConstraint, subpath.fullpath).startsWith('..')
-  })
 
-  return (
+    this.selectFile = this.selectFile.bind(this)
-    <div className={ props.className }>
+  }
-      { dirs.length > MAX_DIR_CRUMBS ? '... / ' : null }
+
-      {
+  render () {
-        _.map(dirs.slice(-MAX_DIR_CRUMBS), (dir, index) => {
+    return (
-          return (
+      <rendition.Button
-            <rendition.Button
+        onClick={ this.selectFile }
-              key={ dir.fullpath }
+        plaintext={ true }>
-              onClick={ () => props.selectFile(dir) }
+        <rendition.Txt bold={ this.props.bold }>
-              plaintext={ true }>
+          { middleEllipsis(labels[this.props.dir.fullpath] || this.props.dir.basename, FILENAME_CHAR_LIMIT_SHORT) }
-              <rendition.Txt bold={ index === dirs.length - 1 }>
+        </rendition.Txt>
-                { middleEllipsis(labels[dir.fullpath] || dir.basename, FILENAME_CHAR_LIMIT_SHORT) }
+      </rendition.Button>
-              </rendition.Txt>
+    )
-            </rendition.Button>
+  }
-          )
+
-        })
+  selectFile () {
-      }
+    this.props.selectFile(this.props.dir)
-    </div>
+  }
-  )
+}
-})`
+
+class UnstyledBreadcrumbs extends React.PureComponent {
+  render () {
+    const folderConstraints = this.props.constraints.length
+      ? this.props.constraints
+      : [ path.parse(this.props.path).root ]
+
+    const dirs = files.subpaths(this.props.path).filter((subpath) => {
+      // Guard against displaying folders outside the constrained folders
+      return folderConstraints.some((folderConstraint) => {
+        return !path.relative(folderConstraint, subpath.fullpath).startsWith('..')
+      })
+    })
+
+    return (
+      <div className={ this.props.className }>
+        { dirs.length > MAX_DIR_CRUMBS ? '... / ' : null }
+        {
+          _.map(dirs.slice(-MAX_DIR_CRUMBS), (dir, index) => {
+            return (
+              <Crumb
+                key={ dir.fullpath }
+                bold={ index === dirs.length - 1 }
+                dir={ dir }
+                selectFile={ this.props.selectFile }
+              />
+            )
+          })
+        }
+      </div>
+    )
+  }
+}
+
+const Breadcrumbs = styled(UnstyledBreadcrumbs)`
   font-size: 18px;
 
   & > button:not(:last-child)::after {
@@ -342,15 +486,12 @@ class FileSelector extends React.PureComponent {
   constructor (props) {
     super(props)
 
-    const fullpath = props.path || os.homedir()
-
     this.state = {
-      path: fullpath,
+      path: props.path,
       files: [],
       history: [],
       highlighted: null,
-      error: null,
+      error: null
-      filters: []
     }
 
     // Filters schema
@@ -370,13 +511,15 @@ class FileSelector extends React.PureComponent {
     }
 
     this.closeModal = this.closeModal.bind(this)
+    this.resolveModal = this.resolveModal.bind(this)
     this.browsePath = this.browsePath.bind(this)
     this.selectFile = this.selectFile.bind(this)
+    this.highlightFile = this.highlightFile.bind(this)
     this.previousDirectory = this.previousDirectory.bind(this)
   }
 
   render () {
-    const items = rendition.SchemaSieve.filter(this.state.filters, this.state.files)
+    const items = this.state.files
 
     const styles = {
       display: 'flex',
@@ -412,26 +555,21 @@ class FileSelector extends React.PureComponent {
             <Breadcrumbs
               path={ this.state.path }
               selectFile={ this.selectFile }
-              constraint={ this.props.constraint }
+              constraints={ this.props.constraints }
             />
           </Header>
           <Main flex="1">
             <Flex direction="column" grow="1">
-              <rendition.Filters
+              <FileListWrap path={ this.state.path }>
-                onFiltersUpdate={ filters => this.setFilters(filters) }
-                onViewsUpdate={ views => this.setViews(views) }
-                schema={ this.schema }
-                renderMode={ [] } />
-
-              <FileListWrap wrap="wrap">
                 {
-                  items.map((item, index) => {
+                  items.map((item) => {
                     return (
-                      <FileLink { ...item }
+                      <FileLink
                         key={ item.fullpath }
+                        file={ item }
                         highlight={ _.get(this.state.highlighted, 'fullpath') === _.get(item, 'fullpath') }
-                        onClick={ () => this.setState({ highlighted: item }) }
+                        highlightFile={ this.highlightFile }
-                        onDoubleClick={ _.partial(this.selectFile, item) }
+                        selectFile={ this.selectFile }
                       />
                     )
                   })
@@ -443,7 +581,7 @@ class FileSelector extends React.PureComponent {
             <rendition.Button onClick={ this.closeModal }>Cancel</rendition.Button>
             <rendition.Button
               primary
-              onClick={ _.partial(this.selectFile, this.state.highlighted) }
+              onClick={ this.resolveModal }
               disabled={ !this.state.highlighted }>
               Select file
             </rendition.Button>
@@ -462,20 +600,21 @@ class FileSelector extends React.PureComponent {
     this.props.close()
   }
 
+  resolveModal () {
+    this.selectFile(this.state.highlighted)
+  }
+
   setFilesProgressively (dirname) {
     return fs.readdirAsync(dirname).then((basenames) => {
-      const fileObjs = basenames.map((basename) => {
-        return {
-          dirname: this.state.path,
-          basename,
-          fullpath: path.join(dirname, basename)
-        }
-      })
-
-      this.setState({ files: fileObjs })
-
       return files.getAllFilesMetadataAsync(dirname, basenames)
     }).then((fileObjs) => {
+      // Sort folders first and ignore case
+      fileObjs.sort((fileA, fileB) => {
+        // NOTE(Shou): the multiplication is an arbitrarily large enough number
+        // to ensure folders have precedence over filenames
+        const directoryPrecedence = (-Number(fileA.isDirectory) + Number(fileB.isDirectory)) * 3
+        return directoryPrecedence + fileA.basename.localeCompare(fileB.basename, undefined, { sensitivity: 'base' })
+      })
       this.setState({ files: fileObjs })
     })
   }
@@ -487,11 +626,16 @@ class FileSelector extends React.PureComponent {
       'dirname'
     ]))
 
-    const folderConstraint = this.props.constraint || path.parse(this.state.path).root
+    const folderConstraints = this.props.constraints.length
+      ? this.props.constraints
+      : [ path.parse(this.props.path).root ]
 
     // Guard against browsing outside the constrained folder
-    if (path.relative(folderConstraint, file.fullpath).startsWith('..')) {
+    const isWithinAnyConstraint = folderConstraints.some((folderConstraint) => {
-      const error = `Cannot browse outside constrained folder ${constrainedFolder}`
+      return !path.relative(folderConstraint, file.fullpath).startsWith('..')
+    })
+    if (!isWithinAnyConstraint) {
+      const error = `Cannot browse outside constrained folders ${folderConstraints}`
       analytics.logException(new Error(error))
       this.setState({ error })
       return
@@ -595,6 +739,10 @@ class FileSelector extends React.PureComponent {
     }
   }
 
+  highlightFile (file) {
+    this.setState({ highlighted: file })
+  }
+
   previousDirectory () {
     analytics.logEvent('Prev directory', null)
     const dir = this.state.history.shift()
@@ -604,14 +752,6 @@ class FileSelector extends React.PureComponent {
       this.browsePath(dir)
     }
   }
-
-  setFilters (filters) {
-    this.setState({ filters })
-  }
-
-  setViews (views) {
-    this.setState({ views })
-  }
 }
 
 FileSelector.propTypes = {
@@ -619,7 +759,7 @@ FileSelector.propTypes = {
 
   close: propTypes.func,
 
-  constraint: propTypes.string
+  constraints: propTypes.arrayOf(propTypes.string)
 }
 
 module.exports = FileSelector

lib/gui/app/components/file-selector/styles/_file-selector.scss

@@ -18,6 +18,6 @@
   width: calc(100vw - 10px);
 
   > .modal-content {
-    height: calc(100vh - 10px);
+    height: calc(100vh - 20px);
   }
 }

lib/gui/app/components/file-selector/templates/file-selector-modal.tpl.html

@@ -1 +1,4 @@
-<file-selector close="selector.close" constraint="selector.getFolderConstraint()" ></file-selector>
+<file-selector
+  constraints="selector.getFolderConstraints()"
+  path="selector.getPath()"
+  close="selector.close"></file-selector>

lib/gui/app/models/files.js

@@ -23,6 +23,31 @@ const fs = Bluebird.promisifyAll(require('fs'))
 
 /* eslint-disable lodash/prefer-lodash-method */
 
+/**
+ * @summary Async exists function
+ * @function
+ * @public
+ *
+ * @description
+ * This is a promise for convenience, as it never fails with an exception/catch.
+ *
+ * @param {String} fullpath - full path
+ * @returns {Boolean}
+ *
+ * @example
+ * files.existsAsync('/home/user/file').then(console.log)
+ * > true
+ */
+exports.existsAsync = (fullpath) => {
+  return new Bluebird((resolve, reject) => {
+    return fs.accessAsync(fullpath, fs.constants.F_OK).then(() => {
+      resolve(true)
+    }).catch(() => {
+      resolve(false)
+    })
+  })
+}
+
 /**
  * @summary Get file metadata
  * @function
@@ -53,7 +78,7 @@ exports.getFileMetadataSync = (dirname, basename = '') => {
     basename: pathObj.base,
     dirname: pathObj.dir,
     fullpath,
-    extension: pathObj.ext.replace('.', ''),
+    extension: pathObj.ext,
     name: pathObj.name,
     isDirectory: stats.isDirectory(),
     isHidden,
@@ -85,7 +110,7 @@ exports.getFileMetadataAsync = (fullpath) => {
       basename: pathObj.base,
       dirname: pathObj.dir,
       fullpath,
-      extension: pathObj.ext.replace('.', ''),
+      extension: pathObj.ext,
       name: pathObj.name,
       isDirectory: stats.isDirectory(),
       isHidden,
@@ -109,15 +134,14 @@ exports.getFileMetadataAsync = (fullpath) => {
  * files.getAllFilesMetadataAsync(os.homedir(), [ 'file1.txt', 'file2.txt' ])
  */
 exports.getAllFilesMetadataAsync = (dirname, basenames) => {
-  return Bluebird.all(basenames.map((basename) => {
+  return Bluebird.reduce(basenames, (fileMetas, basename) => {
-    const metadata = exports.getFileMetadataAsync(path.join(dirname, basename))
+    return new Bluebird((resolve, reject) => {
-    return metadata.reflect()
+      exports.getFileMetadataAsync(path.join(dirname, basename)).then((metadata) => {
-  })).reduce((fileMetas, inspection) => {
+        resolve(fileMetas.concat(metadata))
-    if (inspection.isFulfilled()) {
+      }).catch(() => {
-      return fileMetas.concat(inspection.value())
+        resolve(fileMetas)
-    }
+      })
-
+    })
-    return fileMetas
   }, [])
 }
 

lib/gui/css/main.css

@@ -6505,7 +6505,7 @@ svg-icon {
 .modal-file-selector-modal {
   width: calc(100vw - 10px); }
   .modal-file-selector-modal > .modal-content {
-    height: calc(100vh - 10px); }
+    height: calc(100vh - 20px); }
 
 /*
  * Copyright 2016 resin.io

tests/gui/models/files.spec.js

@@ -18,7 +18,7 @@
 
 const m = require('mochainon')
 const path = require('path')
-const files = require('../../lib/shared/files')
+const files = require('../../../lib/gui/app/models/files')
 
 describe('Shared: Files', function () {
   describe('.splitPath()', function () {