diff --git a/packages/components/src/DraggableItemList.tsx b/packages/components/src/DraggableItemList.tsx index c3e4e0f005..80062de8c5 100644 --- a/packages/components/src/DraggableItemList.tsx +++ b/packages/components/src/DraggableItemList.tsx @@ -64,6 +64,7 @@ class DraggableItemList extends PureComponent< offset: 0, items: [], rowHeight: DraggableItemList.DEFAULT_ROW_HEIGHT, + isDeselectOnClick: true, isDoubleClickSelect: true, isDropDisabled: false, isDragDisabled: false, diff --git a/packages/components/src/ItemList.test.tsx b/packages/components/src/ItemList.test.tsx index edd8b89b96..25c56a3eb9 100644 --- a/packages/components/src/ItemList.test.tsx +++ b/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(); diff --git a/packages/components/src/ItemList.tsx b/packages/components/src/ItemList.tsx index 8465a1a2b6..0799da35a9 100644 --- a/packages/components/src/ItemList.tsx +++ b/packages/components/src/ItemList.tsx @@ -45,6 +45,8 @@ export type ItemListProps = { // Default renderItem will look for a `displayValue` property, fallback // to the `value` property, or stringify the object if neither are defined items: T[]; + // Whether clicking a selected item should deselect in the item list or not. Defaults to true + isDeselectOnClick: boolean; // Whether selection requires a double click or not isDoubleClickSelect: boolean; // Whether to allow dragging to change the selection after clicking @@ -57,7 +59,7 @@ export type ItemListProps = { 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; @@ -98,6 +100,8 @@ export class ItemList extends PureComponent< items: [], rowHeight: ItemList.DEFAULT_ROW_HEIGHT, + isDeselectOnClick: true, + isDoubleClickSelect: false, isDragSelect: true, @@ -362,7 +366,7 @@ export class ItemList 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 +378,7 @@ export class ItemList extends PureComponent< ]), }), () => { - onSelect(itemIndex); + onSelect(itemIndex, e); } ); } @@ -463,14 +467,15 @@ export class ItemList extends PureComponent< this.toggleSelect( itemIndex, e.shiftKey, - ContextActionUtils.isModifierKeyDown(e) + ContextActionUtils.isModifierKeyDown(e), + false ); } } } handleItemMouseUp(index: number, e: React.MouseEvent): void { - const { isDoubleClickSelect, onSelect } = this.props; + const { isDeselectOnClick, isDoubleClickSelect, onSelect } = this.props; const { mouseDownIndex, isDragging } = this.state; if ( @@ -486,10 +491,10 @@ export class ItemList extends PureComponent< const isShiftDown = e.shiftKey; const isModifierDown = ContextActionUtils.isModifierKeyDown(e); this.focusItem(index); - this.toggleSelect(index, isShiftDown, isModifierDown); + this.toggleSelect(index, isShiftDown, isModifierDown, isDeselectOnClick); if (!isDoubleClickSelect && !isShiftDown && !isModifierDown) { - onSelect(index); + onSelect(index, e); } } @@ -522,7 +527,7 @@ export class ItemList extends PureComponent< if (!isMultiSelect && newFocus != null) { this.setState({ selectedRanges: [[newFocus, newFocus]] }, () => { if (newFocus != null) { - onSelect(newFocus); + onSelect(newFocus, e); } }); } diff --git a/packages/dashboard-core-plugins/src/ConsolePlugin.tsx b/packages/dashboard-core-plugins/src/ConsolePlugin.tsx index afc12ee1e4..cb469f4bdd 100644 --- a/packages/dashboard-core-plugins/src/ConsolePlugin.tsx +++ b/packages/dashboard-core-plugins/src/ConsolePlugin.tsx @@ -30,6 +30,7 @@ const log = Log.module('ConsolePlugin'); type NotebookPanelComponent = PanelComponent & { notebook: ScriptEditor; + focus: () => void; }; function isNotebookPanel( @@ -150,7 +151,11 @@ export const ConsolePlugin = ({ [openFileMap, previewFileMap, renamePanel] ); - const activateFilePanel = useCallback( + /** + * Show the panel for the given file metadata. + * If the panel is not already open, then it just logs an error and does nothing. + */ + const showFilePanel = useCallback( fileMetadata => { const panelId = getPanelIdForFileMetadata(fileMetadata, false); if (panelId == null) { @@ -264,14 +269,21 @@ export const ConsolePlugin = ({ [previewFileMap] ); - const fileIsActive = useCallback( - fileMetadata => { - const panelId = getPanelIdForFileMetadata(fileMetadata, false); - return ( - panelId != null && LayoutUtils.isActiveTab(layout.root, { id: panelId }) - ); + /** + * Attempts to focus the panel with the provided panelId + */ + const focusPanelById = useCallback( + panelId => { + if (!panelId) { + return; + } + + const panel = panelManager.getOpenedPanelById(panelId); + if (panel && isNotebookPanel(panel)) { + panel.focus(); + } }, - [getPanelIdForFileMetadata, layout.root] + [panelManager] ); const makeConfig = useCallback( @@ -291,6 +303,7 @@ export const ConsolePlugin = ({ return { type: 'react-component', component: NotebookPanel.COMPONENT, + isFocusOnShow: false, props: { localDashboardId: id, metadata: { id: panelId }, @@ -314,9 +327,10 @@ export const ConsolePlugin = ({ fileMetadata = { id: null, itemName: getNotebookFileName(settings) } ) => { const panelId = getPanelIdForFileMetadata(fileMetadata); - if (fileIsOpen(fileMetadata)) { - log.debug('File is already open, focus tab'); - LayoutUtils.activateTab(layout.root, { id: panelId }); + if (fileIsOpen(fileMetadata) && panelId) { + log.debug('File is already open, focus panel'); + showFilePanel(fileMetadata); + focusPanelById(panelId); return; } const stack = LayoutUtils.getStackForComponentTypes(layout.root, [ @@ -334,44 +348,40 @@ export const ConsolePlugin = ({ }, [ fileIsOpen, + focusPanelById, getNotebookFileName, getPanelIdForFileMetadata, layout.root, makeConfig, openFileMap, + showFilePanel, ] ); 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()); + // If the file is already open, just show and focus it if necessary + if (isFileOpen) { + showFilePanel(fileMetadata); + if (shouldFocus) { + const panelId = getPanelIdForFileMetadata(fileMetadata); + focusPanelById(panelId); } - } 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; } + + // If the file is already open as a preview and we're not focusing it, just show it + // If we're focusing it, that means we need to replace the panel anyway with a non-preview panel, so just fall into the logic below + if (isFileOpenAsPreview && !shouldFocus) { + showFilePanel(fileMetadata); + return; + } + + const [previewTabId] = Array.from(previewFileMap.values()); let panelId = null; let stack = null; if (previewTabId != null) { @@ -393,20 +403,19 @@ 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) { - LayoutUtils.activateTab(layout.root, { id: panelId }); + if (shouldFocus) { + // Focus the tab we just opened if we're supposed to + focusPanelById(panelId); } }, [ - activateFilePanel, - fileIsActive, + showFilePanel, fileIsOpen, fileIsOpenAsPreview, + focusPanelById, getPanelIdForFileMetadata, layout.root, makeConfig, diff --git a/packages/dashboard-core-plugins/src/panels/FileExplorerPanel.tsx b/packages/dashboard-core-plugins/src/panels/FileExplorerPanel.tsx index fd3c41d8ec..96efa0460c 100644 --- a/packages/dashboard-core-plugins/src/panels/FileExplorerPanel.tsx +++ b/packages/dashboard-core-plugins/src/panels/FileExplorerPanel.tsx @@ -36,6 +36,11 @@ export interface FileExplorerPanelState { showCreateFolder: boolean; } +function isMouseEvent(e: React.SyntheticEvent): e is React.MouseEvent { + const mouseEvent = e as React.MouseEvent; + return mouseEvent && typeof mouseEvent.button === 'number'; +} + /** * Panel for showing a FileExplorer in a Dashboard. */ @@ -51,6 +56,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 +147,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 +166,8 @@ export class FileExplorerPanel extends React.Component< session, language, notebookSettings, - fileMetadata + fileMetadata, + shouldFocus ); } diff --git a/packages/dashboard-core-plugins/src/panels/NotebookPanel.jsx b/packages/dashboard-core-plugins/src/panels/NotebookPanel.jsx index 0014abc66d..a4817f18d4 100644 --- a/packages/dashboard-core-plugins/src/panels/NotebookPanel.jsx +++ b/packages/dashboard-core-plugins/src/panels/NotebookPanel.jsx @@ -79,6 +79,7 @@ class NotebookPanel extends Component { this.handleFocus = this.handleFocus.bind(this); this.handleLoadSuccess = this.handleLoadSuccess.bind(this); this.handleLoadError = this.handleLoadError.bind(this); + this.handlePanelTabClick = this.handlePanelTabClick.bind(this); this.handleRenameFile = this.handleRenameFile.bind(this); this.handleResize = this.handleResize.bind(this); this.handleRunCommand = this.handleRunCommand.bind(this); @@ -91,7 +92,6 @@ class NotebookPanel extends Component { this.handleSaveSuccess = this.handleSaveSuccess.bind(this); this.handleSessionOpened = this.handleSessionOpened.bind(this); this.handleSessionClosed = this.handleSessionClosed.bind(this); - this.handleShow = this.handleShow.bind(this); this.handleShowRename = this.handleShowRename.bind(this); this.handleTab = this.handleTab.bind(this); @@ -175,13 +175,12 @@ class NotebookPanel extends Component { } componentDidMount() { - const { - glContainer: { tab }, - glEventHub, - } = this.props; + const { glContainer, glEventHub } = this.props; + const { tab } = glContainer; if (tab) this.initTab(tab); this.initNotebookContent(); glEventHub.on(NotebookEvent.RENAME_FILE, this.handleRenameFile); + glContainer.on('tabClicked', this.handlePanelTabClick); } componentDidUpdate(prevProps, prevState) { @@ -195,10 +194,11 @@ class NotebookPanel extends Component { this.debouncedSavePanelState.flush(); this.pending.cancel(); - const { glEventHub } = this.props; + const { glContainer, glEventHub } = this.props; const { fileMetadata, isPreview } = this.state; glEventHub.off(NotebookEvent.RENAME_FILE, this.handleRenameFile); + glContainer.off('tabClicked', this.handlePanelTabClick); glEventHub.emit(NotebookEvent.UNREGISTER_FILE, fileMetadata, isPreview); } @@ -632,11 +632,6 @@ class NotebookPanel extends Component { return; } this.notebook.updateDimensions(); - requestAnimationFrame(() => { - if (this.notebook) { - this.notebook.focus(); - } - }); } handleShowRename() { @@ -648,15 +643,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(); } } @@ -667,6 +661,19 @@ class NotebookPanel extends Component { }); } + handlePanelTabClick() { + log.debug('handlePanelTabClick'); + this.focus(); + } + + focus() { + requestAnimationFrame(() => { + if (this.notebook) { + this.notebook.focus(); + } + }); + } + createNotebook(itemName, language, content) { const { glEventHub } = this.props; const { session, sessionLanguage, settings } = this.state; diff --git a/packages/dashboard/src/layout/LayoutUtils.js b/packages/dashboard/src/layout/LayoutUtils.js index a88fe6283c..f322293616 100644 --- a/packages/dashboard/src/layout/LayoutUtils.js +++ b/packages/dashboard/src/layout/LayoutUtils.js @@ -54,7 +54,12 @@ class LayoutUtils { if (!isCorrectType) { parent.removeChild(child, true); parent.addChild({ type }); + + // The addChild may cause the element that has focus to be removed from the DOM, which changes focus to the body + // Try and maintain the focus as best we can. The unfocused element may still send a blur/focus event so that needs to be handled correctly. + const maintainFocusElement = document.activeElement; parent.contentItems[0].addChild(child); + maintainFocusElement?.focus(); } return this.addStack(parent.contentItems[0], columnPreferred); diff --git a/packages/file-explorer/src/FileExplorer.tsx b/packages/file-explorer/src/FileExplorer.tsx index 9123c40f61..73068930fd 100644 --- a/packages/file-explorer/src/FileExplorer.tsx +++ b/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; diff --git a/packages/file-explorer/src/FileList.tsx b/packages/file-explorer/src/FileList.tsx index 47372f9ca8..daecc84a06 100644 --- a/packages/file-explorer/src/FileList.tsx +++ b/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.SyntheticEvent) => { 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); } @@ -560,6 +560,7 @@ export const FileList = (props: FileListProps): JSX.Element => { rowHeight={rowHeight} isMultiSelect={isMultiSelect} isDragSelect={false} + isDeselectOnClick={false} /> ); diff --git a/packages/file-explorer/src/FileListContainer.tsx b/packages/file-explorer/src/FileListContainer.tsx index f7762bb1b5..2ced464949 100644 --- a/packages/file-explorer/src/FileListContainer.tsx +++ b/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; /** Height of each item in the list */ diff --git a/packages/golden-layout/src/js/config/ItemDefaultConfig.js b/packages/golden-layout/src/js/config/ItemDefaultConfig.js index 2098416ca1..14ff021121 100644 --- a/packages/golden-layout/src/js/config/ItemDefaultConfig.js +++ b/packages/golden-layout/src/js/config/ItemDefaultConfig.js @@ -1,5 +1,6 @@ lm.config.itemDefaultConfig = { isClosable: true, + isFocusOnShow: true, reorderEnabled: true, title: '', }; diff --git a/packages/golden-layout/src/js/controls/Tab.js b/packages/golden-layout/src/js/controls/Tab.js index 5314ff08f0..17df4dca95 100644 --- a/packages/golden-layout/src/js/controls/Tab.js +++ b/packages/golden-layout/src/js/controls/Tab.js @@ -224,6 +224,7 @@ lm.utils.copy(lm.controls.Tab.prototype, { var activeContentItem = this.header.parent.getActiveContentItem(); if (this.contentItem !== activeContentItem) { this.header.parent.setActiveContentItem(this.contentItem); + this.contentItem.container.emit('tabClicked'); } else if ( this.contentItem.isComponent && !this.contentItem.container._contentElement[0].contains( diff --git a/packages/golden-layout/src/js/items/Component.js b/packages/golden-layout/src/js/items/Component.js index 8192b34f9a..2166db8416 100644 --- a/packages/golden-layout/src/js/items/Component.js +++ b/packages/golden-layout/src/js/items/Component.js @@ -59,17 +59,14 @@ lm.utils.copy(lm.items.Component.prototype, { _$show: function () { this.container.show(); - // focus the shown container element on show - // preventScroll isn't supported in safari, but also doesn't matter for illumon when 100% window - this.container._contentElement[0].focus({ preventScroll: true }); + if (this.container._config.isFocusOnShow) { + // focus the shown container element on show + // preventScroll isn't supported in safari, but also doesn't matter for illumon when 100% window + this.container._contentElement[0].focus({ preventScroll: true }); + } lm.items.AbstractContentItem.prototype._$show.call(this); }, - _$shown: function () { - this.container.shown(); - lm.items.AbstractContentItem.prototype._$shown.call(this); - }, - _$destroy: function () { this.container.emit('destroy', this); lm.items.AbstractContentItem.prototype._$destroy.call(this);