From b30abd70c756a76157cf9f8ac4357728bc3068f0 Mon Sep 17 00:00:00 2001
From: Shawn Erquhart <shawn@erquh.art>
Date: Wed, 30 Aug 2017 21:31:03 -0400
Subject: [PATCH] fix nested dialog focus trap, improve media table

---
 src/components/MediaLibrary/FocusTrap.js     | 16 ++++++++
 src/components/MediaLibrary/MediaLibrary.css |  2 +-
 src/components/MediaLibrary/MediaLibrary.js  | 41 +++++++++++---------
 3 files changed, 40 insertions(+), 19 deletions(-)
 create mode 100644 src/components/MediaLibrary/FocusTrap.js

diff --git a/src/components/MediaLibrary/FocusTrap.js b/src/components/MediaLibrary/FocusTrap.js
new file mode 100644
index 000000000000..86c284932619
--- /dev/null
+++ b/src/components/MediaLibrary/FocusTrap.js
@@ -0,0 +1,16 @@
+import React from 'react';
+import FocusTrapReact from 'focus-trap-react';
+
+/**
+ * A wrapper for focus-trap-react, which we use to completely remove focus traps
+ * from the DOM rather than using the library's own internal activation/pausing
+ * mechanisms, which can manifest bugs when nested.
+ */
+const FocusTrap = props => {
+  const { active, children, focusTrapOptions, className } = props;
+  return active
+    ? <FocusTrapReact focusTrapOptions={focusTrapOptions} className={className}>{children}</FocusTrapReact>
+    : <div className={className}>{children}</div>
+}
+
+export default FocusTrap;
diff --git a/src/components/MediaLibrary/MediaLibrary.css b/src/components/MediaLibrary/MediaLibrary.css
index 4c0e8b1c4e40..ebc605566601 100644
--- a/src/components/MediaLibrary/MediaLibrary.css
+++ b/src/components/MediaLibrary/MediaLibrary.css
@@ -50,7 +50,7 @@
 
 .thumbnail {
   max-height: 44px;
-  max-width: 120px;
+  max-width: 88px;
   margin: -10px auto;
   display: block;
   border-radius: 4px;
diff --git a/src/components/MediaLibrary/MediaLibrary.js b/src/components/MediaLibrary/MediaLibrary.js
index 2ffc0f8ce3ee..6d1d54f3cfb0 100644
--- a/src/components/MediaLibrary/MediaLibrary.js
+++ b/src/components/MediaLibrary/MediaLibrary.js
@@ -6,7 +6,6 @@ import { Table, TableHead, TableRow, TableCell } from 'react-toolbox/lib/table';
 import { Button, BrowseButton } from 'react-toolbox/lib/button';
 import Overlay from 'react-toolbox/lib/overlay';
 import ProgressBar from 'react-toolbox/lib/progress_bar';
-import FocusTrap from 'focus-trap-react';
 import bytes from 'bytes';
 import fuzzy from 'fuzzy';
 import { resolvePath } from '../../lib/pathHelper';
@@ -14,6 +13,7 @@ import { createAssetProxy } from '../../valueObjects/AssetProxy';
 import { changeDraftField } from '../../actions/entries';
 import { addAsset } from '../../actions/media';
 import { loadMedia, persistMedia, deleteMedia, insertMedia, closeMediaLibrary } from '../../actions/mediaLibrary';
+import FocusTrap from './FocusTrap';
 import styles from './MediaLibrary.css';
 import dialogTheme from './dialogTheme.css';
 import headCellTheme from './headCellTheme.css';
@@ -226,27 +226,30 @@ class MediaLibrary extends React.Component {
         theme={dialogTheme}
         className={styles.dialog}
       >
-        <Overlay active={shouldShowProgressBar} theme={progressOverlayTheme}>
-          <FocusTrap
-            paused={!isVisible || !shouldShowProgressBar}
-            focusTrapOptions={{ clickOutsideDeactivates: true, initialFocus: 'h1' }}
-            className={styles.progressBarContainer}
-          >
-            <h1 style={{ marginTop: '-40px' }} tabIndex="-1">{ loadingMessage }</h1>
-            <ProgressBar type="linear" mode="indeterminate" theme={progressBarTheme}/>
-          </FocusTrap>
-        </Overlay>
         <FocusTrap
-          paused={!isVisible || shouldShowProgressBar}
+          active={isVisible && !shouldShowProgressBar}
           focusTrapOptions={{ clickOutsideDeactivates: true }}
           className={styles.tableContainer}
         >
+          <Overlay active={shouldShowProgressBar} theme={progressOverlayTheme}>
+            <FocusTrap
+              active={isVisible && shouldShowProgressBar}
+              focusTrapOptions={{ clickOutsideDeactivates: true, initialFocus: 'h1' }}
+              className={styles.progressBarContainer}
+            >
+              <h1 style={{ marginTop: '-40px' }} tabIndex="-1">{ loadingMessage }</h1>
+              <ProgressBar type="linear" mode="indeterminate" theme={progressBarTheme}/>
+            </FocusTrap>
+          </Overlay>
           <h1>{forImage ? 'Images' : 'Assets'}</h1>
           <input className={styles.searchInput} type="text" value={this.state.query} onChange={this.handleSearch.bind(this)} placeholder="Search..." disabled={!hasFiles || !hasImages} autoFocus/>
           <div style={{ height: '100%', paddingBottom: '130px' }}>
             <div style={{ height: '100%', overflowY: 'auto' }} ref={ref => this.tableScrollRef = ref}>
               <Table onRowSelect={idx => this.handleRowSelect(tableData[idx])}>
                 <TableHead>
+                  <TableCell theme={headCellTheme} style={{ width: '92px' }}>
+                    Image
+                  </TableCell>
                   <TableCell
                     theme={headCellTheme}
                     sorted={hasMedia && this.getSortDirection('name')}
@@ -271,19 +274,21 @@ class MediaLibrary extends React.Component {
                   >
                     Size
                   </TableCell>
-                  <TableCell theme={headCellTheme}>
-                    Image
-                  </TableCell>
                 </TableHead>
                 {
                   tableData.map((file, idx) =>
                     <TableRow key={idx} selected={this.state.selectedFileName === file.name } onFocus={this.handleRowFocus} onBlur={this.handleRowBlur}>
+                      <TableCell>
+                        {
+                          !file.isImage ? null :
+                            <a href={file.url} target="_blank">
+                              <img src={file.url} className={styles.thumbnail}/>
+                            </a>
+                        }
+                      </TableCell>
                       <TableCell>{file.name}</TableCell>
                       <TableCell>{file.type}</TableCell>
                       <TableCell>{bytes(file.size, { decimalPlaces: 0 })}</TableCell>
-                      <TableCell>
-                        {file.isImage ? <img src={file.url} className={styles.thumbnail}/> : null}
-                      </TableCell>
                     </TableRow>
                   )
                 }