Fix unexpected click handling in File Explorer

packages/components/src/ItemList.test.tsx

@@ -86,7 +86,7 @@ describe('mouse', () => {
 
     clickItem(itemList, 3);
 
-    expect(onSelect).toHaveBeenCalledWith(3);
+    expect(onSelect).toHaveBeenCalledWith(3, expect.anything());
 
     itemList.unmount();
   });
@@ -101,7 +101,7 @@ describe('mouse', () => {
 
     doubleClickItem(itemList, 3);
 
-    expect(onSelect).toHaveBeenCalledWith(3);
+    expect(onSelect).toHaveBeenCalledWith(3, expect.anything());
 
     itemList.unmount();
   });
@@ -117,7 +117,7 @@ describe('mouse', () => {
 
     clickItem(itemList, 3);
 
-    expect(onSelect).toHaveBeenCalledWith(3);
+    expect(onSelect).toHaveBeenCalledWith(3, expect.anything());
     expect(onSelectionChange).toHaveBeenCalledWith([[3, 3]]);
 
     onSelectionChange.mockClear();
@@ -142,7 +142,7 @@ describe('mouse', () => {
 
     clickItem(itemList, 3);
 
-    expect(onSelect).toHaveBeenCalledWith(3);
+    expect(onSelect).toHaveBeenCalledWith(3, expect.anything());
     expect(onSelectionChange).toHaveBeenCalledWith([[3, 3]]);
 
     onSelectionChange.mockClear();
@@ -176,7 +176,7 @@ describe('mouse', () => {
 
       clickItem(itemList, firstIndex);
 
-      expect(onSelect).toHaveBeenCalledWith(firstIndex);
+      expect(onSelect).toHaveBeenCalledWith(firstIndex, expect.anything());
       expect(onSelectionChange).toHaveBeenCalledWith([
         [firstIndex, firstIndex],
       ]);
@@ -231,7 +231,7 @@ describe('mouse', () => {
 
       clickItem(itemList, 3);
 
-      expect(onSelect).toHaveBeenCalledWith(3);
+      expect(onSelect).toHaveBeenCalledWith(3, expect.anything());
       expect(onSelectionChange).toHaveBeenCalledWith([[3, 3]]);
 
       onSelectionChange.mockClear();

packages/components/src/ItemList.tsx

@@ -57,7 +57,7 @@ export type ItemListProps<T> = {
   onFocusChange(index: number | null): void;
 
   // Fired when an item is clicked. With multiple selection, fired on double click.
-  onSelect(index: number): void;
+  onSelect(index: number, event: React.SyntheticEvent): void;
   onSelectionChange(ranges: Range[]): void;
   onViewportChange(topRow: number, bottomRow: number): void;
   overscanCount: number;
@@ -362,7 +362,7 @@ export class ItemList<T> extends PureComponent<
     this.toggleSelect(itemIndex, e.shiftKey, isModifierDown, false);
   }
 
-  handleItemDoubleClick(itemIndex: number): void {
+  handleItemDoubleClick(itemIndex: number, e: React.MouseEvent): void {
     const { isDoubleClickSelect, onSelect } = this.props;
 
     if (isDoubleClickSelect) {
@@ -374,7 +374,7 @@ export class ItemList<T> extends PureComponent<
           ]),
         }),
         () => {
-          onSelect(itemIndex);
+          onSelect(itemIndex, e);
         }
       );
     }
@@ -489,7 +489,7 @@ export class ItemList<T> extends PureComponent<
       this.toggleSelect(index, isShiftDown, isModifierDown);
 
       if (!isDoubleClickSelect && !isShiftDown && !isModifierDown) {
-        onSelect(index);
+        onSelect(index, e);
       }
     }
 
@@ -522,7 +522,7 @@ export class ItemList<T> extends PureComponent<
       if (!isMultiSelect && newFocus != null) {
         this.setState({ selectedRanges: [[newFocus, newFocus]] }, () => {
           if (newFocus != null) {
-            onSelect(newFocus);
+            onSelect(newFocus, e);
           }
         });
       }

packages/dashboard-core-plugins/src/ConsolePlugin.tsx

@@ -264,16 +264,6 @@ export const ConsolePlugin = ({
     [previewFileMap]
   );
 
-  const fileIsActive = useCallback(
-    fileMetadata => {
-      const panelId = getPanelIdForFileMetadata(fileMetadata, false);
-      return (
-        panelId != null && LayoutUtils.isActiveTab(layout.root, { id: panelId })
-      );
-    },
-    [getPanelIdForFileMetadata, layout.root]
-  );
-
   const makeConfig = useCallback(
     ({
       id: panelId,
@@ -343,35 +333,24 @@ export const ConsolePlugin = ({
   );
 
   const selectNotebook = useCallback(
-    (session, sessionLanguage, settings, fileMetadata) => {
-      log.debug('selectNotebook', fileMetadata);
-      let previewTabId = null;
-      let isPreview = true;
+    (session, sessionLanguage, settings, fileMetadata, shouldFocus = false) => {
+      log.debug('selectNotebook', fileMetadata, shouldFocus);
       const isFileOpen = fileIsOpen(fileMetadata);
       const isFileOpenAsPreview = fileIsOpenAsPreview(fileMetadata);
-
-      if (!isFileOpen && !isFileOpenAsPreview) {
-        log.debug('selectNotebook, file not open');
-        if (previewFileMap.size > 0) {
-          log.debug('selectNotebook, file not open, previewFileMap not empty');
-          // Existing preview tab id to reuse
-          [previewTabId] = Array.from(previewFileMap.values());
+      const maintainFocusElement = document.activeElement;
+
+      if (isFileOpen || (isFileOpenAsPreview && !shouldFocus)) {
+        activateFilePanel(fileMetadata);
+        if (!shouldFocus) {
+          // Need to request the animation frame because it may have been in the background before being shown
+          requestAnimationFrame(() => {
+            (maintainFocusElement as HTMLElement)?.focus();
+          });
         }
-      } else {
-        // File already open in background
-        if (!fileIsActive(fileMetadata)) {
-          activateFilePanel(fileMetadata);
-          return;
-        }
-        // File already open in foreground, not in preview mode
-        if (!isFileOpenAsPreview) {
-          activateFilePanel(fileMetadata);
-          return;
-        }
-        // File already open in foreground in preview mode
-        [previewTabId] = Array.from(previewFileMap.values());
-        isPreview = false;
+        return;
       }
+
+      const [previewTabId] = Array.from(previewFileMap.values());
       let panelId = null;
       let stack = null;
       if (previewTabId != null) {
@@ -393,18 +372,17 @@ export const ConsolePlugin = ({
         fileMetadata,
         session,
         sessionLanguage,
-        isPreview,
+        isPreview: !shouldFocus,
       });
       LayoutUtils.openComponentInStack(stack, config);
       // openComponentInStack attempts to maintain the focus
       // Focus open tab if it's editable
-      if (!isPreview) {
+      if (shouldFocus) {
         LayoutUtils.activateTab(layout.root, { id: panelId });
       }
     },
     [
       activateFilePanel,
-      fileIsActive,
       fileIsOpen,
       fileIsOpenAsPreview,
       getPanelIdForFileMetadata,

packages/dashboard-core-plugins/src/panels/FileExplorerPanel.tsx

@@ -36,6 +36,15 @@ export interface FileExplorerPanelState {
   showCreateFolder: boolean;
 }
 
+function isMouseEvent<T>(e: React.SyntheticEvent<T>): e is React.MouseEvent<T> {
+  const mouseEvent = e as React.MouseEvent<T>;
+  return (
+    mouseEvent &&
+    typeof mouseEvent.pageX === 'number' &&
+    typeof mouseEvent.pageY === 'number'
+  );
+}
+
 /**
  * Panel for showing a FileExplorer in a Dashboard.
  */
@@ -51,6 +60,20 @@ export class FileExplorerPanel extends React.Component<
     log.error(error);
   }
 
+  /**
+   * Return true if the event should open a file and focus it, otherwise false
+   * @param event The event to check
+   */
+  static isFocusEvent(event: React.SyntheticEvent): boolean {
+    if (isMouseEvent(event)) {
+      // If it's not a double click, then it's a preview event
+      return event.detail >= 2;
+    }
+
+    // Keyboard event should always open with focus
+    return true;
+  }
+
   constructor(props: FileExplorerPanelProps) {
     super(props);
 
@@ -128,12 +151,13 @@ export class FileExplorerPanel extends React.Component<
     });
   }
 
-  handleFileSelect(file: FileStorageItem): void {
+  handleFileSelect(file: FileStorageItem, event: React.SyntheticEvent): void {
     log.debug('fileSelect', file);
     if (file.type === 'directory') {
       return;
     }
 
+    const shouldFocus = FileExplorerPanel.isFocusEvent(event);
     const fileMetadata = { id: file.filename, itemName: file.filename };
     const { glEventHub } = this.props;
     const { session, language } = this.state;
@@ -146,7 +170,8 @@ export class FileExplorerPanel extends React.Component<
       session,
       language,
       notebookSettings,
-      fileMetadata
+      fileMetadata,
+      shouldFocus
     );
   }
 

packages/dashboard-core-plugins/src/panels/NotebookPanel.jsx

@@ -632,6 +632,7 @@ class NotebookPanel extends Component {
       return;
     }
     this.notebook.updateDimensions();
+
     requestAnimationFrame(() => {
       if (this.notebook) {
         this.notebook.focus();
@@ -648,15 +649,14 @@ class NotebookPanel extends Component {
     this.initTab(tab);
   }
 
-  handleTabFocus() {
-    log.debug('handleTabFocus');
+  handleTabFocus(...args) {
+    log.debug('handleTabFocus', ...args);
     const { glContainer } = this.props;
     this.setState({
       isDashboardActive: true,
     });
     if (this.notebook && !glContainer.isHidden) {
       this.notebook.updateDimensions();
-      this.notebook.focus();
     }
   }
 

packages/file-explorer/src/FileExplorer.tsx

@@ -23,7 +23,7 @@ export interface FileExplorerProps {
 
   onDelete?: (files: FileStorageItem[]) => void;
   onRename?: (oldName: string, newName: string) => void;
-  onSelect: (file: FileStorageItem) => void;
+  onSelect: (file: FileStorageItem, event: React.SyntheticEvent) => void;
 
   /** Height of each item in the list */
   rowHeight?: number;

packages/file-explorer/src/FileList.tsx

@@ -49,7 +49,7 @@ export interface FileListProps {
 
   onFocusChange?: (focusedItem?: FileStorageItem) => void;
   onMove: (files: FileStorageItem[], path: string) => void;
-  onSelect: (file: FileStorageItem) => void;
+  onSelect: (file: FileStorageItem, event: React.SyntheticEvent) => void;
   onSelectionChange?: (selectedItems: FileStorageItem[]) => void;
 
   renderItem?: (props: FileListRenderItemProps) => JSX.Element;
@@ -307,12 +307,12 @@ export const FileList = (props: FileListProps): JSX.Element => {
   );
 
   const handleSelect = useCallback(
-    (itemIndex: number) => {
+    (itemIndex: number, event: React.UIEvent) => {
       const item = loadedViewport.items[itemIndex - loadedViewport.offset];
       if (item !== undefined) {
         log.debug('handleItemClick', item);
 
-        onSelect(item);
+        onSelect(item, event);
         if (isDirectory(item)) {
           table?.setExpanded(item.filename, !item.isExpanded);
         }

packages/file-explorer/src/FileListContainer.tsx

@@ -23,7 +23,7 @@ export interface FileListContainerProps {
   onDelete?: (files: FileStorageItem[]) => void;
   onMove?: (files: FileStorageItem[], path: string) => void;
   onRename?: (file: FileStorageItem, newName: string) => void;
-  onSelect: (file: FileStorageItem) => void;
+  onSelect: (file: FileStorageItem, event: React.SyntheticEvent) => void;
   validateRename?: (file: FileStorageItem, newName: string) => Promise<void>;
 
   /** Height of each item in the list */