Remove root level flags used to track cell selection and focus (#9882)

For #9340
To be merged after #9840

Basically I got rid of the root level state info that keeps track of the cell that's selected and focused.
This required us to keep the two in sync along with the state at the cell level.
When syncing information between multiple editors this got messy. removing this removes the issues where things could go out of sync, i.e. two ways to storing the same thing. Now we get selected information from the cells directly - single source of truth.

Author: DonJayamanne

src/datascience-ui/history-react/redux/reducers/creation.ts

@@ -120,9 +120,7 @@ export namespace Creation {
         return {
             ...arg.prevState,
             cellVMs: [],
-            undoStack: Helpers.pushStack(arg.prevState.undoStack, arg.prevState.cellVMs),
-            selectedCellId: undefined,
-            focusedCellId: undefined
+            undoStack: Helpers.pushStack(arg.prevState.undoStack, arg.prevState.cellVMs)
         };
     }
 

src/datascience-ui/history-react/redux/reducers/execution.ts

@@ -24,16 +24,13 @@ export namespace Execution {
             const cells = arg.prevState.undoStack[arg.prevState.undoStack.length - 1];
             const undoStack = arg.prevState.undoStack.slice(0, arg.prevState.undoStack.length - 1);
             const redoStack = Helpers.pushStack(arg.prevState.redoStack, arg.prevState.cellVMs);
-            const selected = cells.findIndex(c => c.selected);
             arg.queueAction(createPostableAction(InteractiveWindowMessages.Undo));
             return {
                 ...arg.prevState,
                 cellVMs: cells,
                 undoStack: undoStack,
                 redoStack: redoStack,
-                skipNextScroll: true,
-                selectedCellId: selected >= 0 ? cells[selected].cell.id : undefined,
-                focusedCellId: selected >= 0 && cells[selected].focused ? cells[selected].cell.id : undefined
+                skipNextScroll: true
             };
         }
 
@@ -46,16 +43,13 @@ export namespace Execution {
             const cells = arg.prevState.redoStack[arg.prevState.redoStack.length - 1];
             const redoStack = arg.prevState.redoStack.slice(0, arg.prevState.redoStack.length - 1);
             const undoStack = Helpers.pushStack(arg.prevState.undoStack, arg.prevState.cellVMs);
-            const selected = cells.findIndex(c => c.selected);
             arg.queueAction(createPostableAction(InteractiveWindowMessages.Redo));
             return {
                 ...arg.prevState,
                 cellVMs: cells,
                 undoStack: undoStack,
                 redoStack: redoStack,
-                skipNextScroll: true,
-                selectedCellId: selected >= 0 ? cells[selected].cell.id : undefined,
-                focusedCellId: selected >= 0 && cells[selected].focused ? cells[selected].cell.id : undefined
+                skipNextScroll: true
             };
         }
 

src/datascience-ui/interactive-common/mainState.ts

@@ -60,8 +60,6 @@ export type IMainState = {
     currentExecutionCount: number;
     debugging: boolean;
     dirty?: boolean;
-    selectedCellId?: string;
-    focusedCellId?: string;
     isAtBottom: boolean;
     newCellId?: string;
     loadTotal?: number;
@@ -75,6 +73,29 @@ export type IMainState = {
     kernel: IServerState;
 };
 
+/**
+ * Returns the cell id and index of selected and focused cells.
+ */
+export function getSelectedAndFocusedInfo(state: IMainState) {
+    const info: { selectedCellId?: string; selectedCellIndex?: number; focusedCellId?: string; focusedCellIndex?: number } = {};
+    for (let index = 0; index < state.cellVMs.length; index += 1) {
+        const cell = state.cellVMs[index];
+        if (cell.selected) {
+            info.selectedCellId = cell.cell.id;
+            info.selectedCellIndex = index;
+        }
+        if (cell.focused) {
+            info.focusedCellId = cell.cell.id;
+            info.focusedCellIndex = index;
+        }
+        if (info.selectedCellId && info.focusedCellId) {
+            break;
+        }
+    }
+
+    return info;
+}
+
 export interface IFont {
     size: number;
     family: string;

src/datascience-ui/interactive-common/redux/reducers/transfer.ts

@@ -3,7 +3,7 @@
 'use strict';
 import { InteractiveWindowMessages } from '../../../../client/datascience/interactive-common/interactiveWindowTypes';
 import { CssMessages } from '../../../../client/datascience/messages';
-import { extractInputText, IMainState } from '../../mainState';
+import { extractInputText, getSelectedAndFocusedInfo, IMainState } from '../../mainState';
 import { createPostableAction } from '../postOffice';
 import { Helpers } from './helpers';
 import { CommonActionType, CommonReducerArg, ICellAction, IEditCellAction, ILinkClickAction, ISendCommandAction, IShowDataViewerAction } from './types';
@@ -99,7 +99,8 @@ export namespace Transfer {
             // We keep this saved here so we don't re-render and we put this code into the input / code data
             // when focus is lost
             const index = arg.prevState.cellVMs.findIndex(c => c.cell.id === arg.payload.data.cellId);
-            if (index >= 0 && arg.prevState.focusedCellId === arg.payload.data.cellId) {
+            const selectionInfo = getSelectedAndFocusedInfo(arg.prevState);
+            if (index >= 0 && selectionInfo.focusedCellId === arg.payload.data.cellId) {
                 const newVMs = [...arg.prevState.cellVMs];
                 const current = arg.prevState.cellVMs[index];
                 const newCell = {

src/datascience-ui/interactive-common/redux/store.ts

@@ -9,7 +9,7 @@ import { InteractiveWindowMessages } from '../../../client/datascience/interacti
 import { BaseReduxActionPayload } from '../../../client/datascience/interactive-common/types';
 import { CssMessages } from '../../../client/datascience/messages';
 import { CellState } from '../../../client/datascience/types';
-import { IMainState, ServerStatus } from '../../interactive-common/mainState';
+import { getSelectedAndFocusedInfo, IMainState, ServerStatus } from '../../interactive-common/mainState';
 import { getLocString } from '../../react-common/locReactSide';
 import { PostOffice } from '../../react-common/postOffice';
 import { combineReducers, createQueueableActionMiddleware, QueuableAction } from '../../react-common/reduxUtils';
@@ -73,9 +73,10 @@ function createSendInfoMiddleware(): Redux.Middleware<{}, IStore> {
         const afterState = store.getState();
 
         // If cell vm count changed or selected cell changed, send the message
+        const currentSelection = getSelectedAndFocusedInfo(afterState.main);
         if (
             prevState.main.cellVMs.length !== afterState.main.cellVMs.length ||
-            prevState.main.selectedCellId !== afterState.main.selectedCellId ||
+            getSelectedAndFocusedInfo(prevState.main).selectedCellId !== currentSelection.selectedCellId ||
             prevState.main.undoStack.length !== afterState.main.undoStack.length ||
             prevState.main.redoStack.length !== afterState.main.redoStack.length
         ) {
@@ -84,7 +85,7 @@ function createSendInfoMiddleware(): Redux.Middleware<{}, IStore> {
                     cellCount: afterState.main.cellVMs.length,
                     undoCount: afterState.main.undoStack.length,
                     redoCount: afterState.main.redoStack.length,
-                    selectedCell: afterState.main.selectedCellId
+                    selectedCell: currentSelection.selectedCellId
                 })
             );
         }
@@ -112,12 +113,14 @@ function createTestMiddleware(): Redux.Middleware<{}, IStore> {
         };
 
         // Special case for focusing a cell
-        if (prevState.main.focusedCellId !== afterState.main.focusedCellId && afterState.main.focusedCellId) {
+        const previousSelection = getSelectedAndFocusedInfo(prevState.main);
+        const currentSelection = getSelectedAndFocusedInfo(afterState.main);
+        if (previousSelection.focusedCellId !== currentSelection.focusedCellId && currentSelection.focusedCellId) {
             // Send async so happens after render state changes (so our enzyme wrapper is up to date)
             sendMessage(InteractiveWindowMessages.FocusedCellEditor, { cellId: action.payload.cellId });
         }
         // Special case for unfocusing a cell
-        if (prevState.main.focusedCellId !== afterState.main.focusedCellId && !afterState.main.focusedCellId) {
+        if (previousSelection.focusedCellId !== currentSelection.focusedCellId && !currentSelection.focusedCellId) {
             // Send async so happens after render state changes (so our enzyme wrapper is up to date)
             sendMessage(InteractiveWindowMessages.UnfocusedCellEditor);
         }

src/datascience-ui/native-editor/nativeEditor.tsx

@@ -9,7 +9,7 @@ import { concatMultilineStringInput } from '../common';
 import { ContentPanel, IContentPanelProps } from '../interactive-common/contentPanel';
 import { handleLinkClick } from '../interactive-common/handlers';
 import { KernelSelection } from '../interactive-common/kernelSelection';
-import { ICellViewModel, IMainState } from '../interactive-common/mainState';
+import { getSelectedAndFocusedInfo, ICellViewModel, IMainState } from '../interactive-common/mainState';
 import { IMainWithVariables, IStore } from '../interactive-common/redux/store';
 import { IVariablePanelProps, VariablePanel } from '../interactive-common/variablePanel';
 import { getOSType } from '../react-common/constants';
@@ -109,7 +109,7 @@ export class NativeEditor extends React.Component<INativeEditorProps> {
     // tslint:disable: react-this-binding-issue
     // tslint:disable-next-line: max-func-body-length
     private renderToolbarPanel() {
-        const selectedIndex = this.props.cellVMs.findIndex(c => c.cell.id === this.props.selectedCellId);
+        const selectedInfo = getSelectedAndFocusedInfo(this.props);
 
         const addCell = () => {
             this.props.addCell();
@@ -132,16 +132,16 @@ export class NativeEditor extends React.Component<INativeEditorProps> {
             ? getLocString('DataScience.collapseVariableExplorerTooltip', 'Hide variables active in jupyter kernel')
             : getLocString('DataScience.expandVariableExplorerTooltip', 'Show variables active in jupyter kernel');
         const runAbove = () => {
-            if (this.props.selectedCellId) {
-                this.props.executeAbove(this.props.selectedCellId);
+            if (selectedInfo.selectedCellId) {
+                this.props.executeAbove(selectedInfo.selectedCellId);
                 this.props.sendCommand(NativeCommandType.RunAbove, 'mouse');
             }
         };
         const runBelow = () => {
-            if (this.props.selectedCellId) {
+            if (selectedInfo.selectedCellId && typeof selectedInfo.selectedCellIndex === 'number') {
                 // tslint:disable-next-line: no-suspicious-comment
                 // TODO: Is the source going to be up to date during run below?
-                this.props.executeCellAndBelow(this.props.selectedCellId, concatMultilineStringInput(this.props.cellVMs[selectedIndex].cell.data.source));
+                this.props.executeCellAndBelow(selectedInfo.selectedCellId, concatMultilineStringInput(this.props.cellVMs[selectedInfo.selectedCellIndex].cell.data.source));
                 this.props.sendCommand(NativeCommandType.RunBelow, 'mouse');
             }
         };
@@ -153,8 +153,8 @@ export class NativeEditor extends React.Component<INativeEditorProps> {
             this.props.selectServer();
             this.props.sendCommand(NativeCommandType.SelectServer, 'mouse');
         };
-        const canRunAbove = selectedIndex > 0;
-        const canRunBelow = selectedIndex < this.props.cellVMs.length - 1 && this.props.selectedCellId;
+        const canRunAbove = (selectedInfo.selectedCellIndex ?? -1) > 0;
+        const canRunBelow = (selectedInfo.selectedCellIndex ?? -1) < this.props.cellVMs.length - 1 && selectedInfo.selectedCellId;
 
         return (
             <div id="toolbar-panel">
@@ -315,7 +315,7 @@ export class NativeEditor extends React.Component<INativeEditorProps> {
             }
             case 'z':
             case 'Z':
-                if (this.props.focusedCellId === undefined) {
+                if (!getSelectedAndFocusedInfo(this.props).focusedCellId) {
                     if (event.shiftKey && !event.ctrlKey && !event.altKey) {
                         event.stopPropagation();
                         this.props.redo();

src/datascience-ui/native-editor/redux/reducers/creation.ts

@@ -4,7 +4,7 @@
 
 import { ILoadAllCells, InteractiveWindowMessages } from '../../../../client/datascience/interactive-common/interactiveWindowTypes';
 import { ICell, IDataScienceExtraSettings } from '../../../../client/datascience/types';
-import { createCellVM, createEmptyCell, CursorPos, extractInputText, ICellViewModel, IMainState } from '../../../interactive-common/mainState';
+import { createCellVM, createEmptyCell, CursorPos, extractInputText, getSelectedAndFocusedInfo, ICellViewModel, IMainState } from '../../../interactive-common/mainState';
 import { createPostableAction } from '../../../interactive-common/redux/postOffice';
 import { Helpers } from '../../../interactive-common/redux/reducers/helpers';
 import { IAddCellAction, ICellAction } from '../../../interactive-common/redux/reducers/types';
@@ -113,7 +113,10 @@ export namespace Creation {
 
     export function addNewCell(arg: NativeEditorReducerArg<IAddCellAction>): IMainState {
         // Do the same thing that an insertBelow does using the currently selected cell.
-        return insertBelow({ ...arg, payload: { ...arg.payload, data: { cellId: arg.prevState.selectedCellId, newCellId: arg.payload.data.newCellId } } });
+        return insertBelow({
+            ...arg,
+            payload: { ...arg.payload, data: { cellId: getSelectedAndFocusedInfo(arg.prevState).selectedCellId, newCellId: arg.payload.data.newCellId } }
+        });
     }
 
     export function startCell(arg: NativeEditorReducerArg<ICell>): IMainState {
@@ -152,9 +155,7 @@ export namespace Creation {
         return {
             ...arg.prevState,
             cellVMs: [newVM],
-            undoStack: Helpers.pushStack(arg.prevState.undoStack, arg.prevState.cellVMs),
-            selectedCellId: undefined,
-            focusedCellId: undefined
+            undoStack: Helpers.pushStack(arg.prevState.undoStack, arg.prevState.cellVMs)
         };
     }
 
@@ -194,23 +195,18 @@ export namespace Creation {
                 arg.queueAction(createPostableAction(InteractiveWindowMessages.RemoveCell, { id: arg.payload.data.cellId }));
 
                 // Recompute select/focus if this item has either
-                let newSelection = arg.prevState.selectedCellId;
-                let newFocused = arg.prevState.focusedCellId;
+                const previousSelection = getSelectedAndFocusedInfo(arg.prevState);
                 const newVMs = [...arg.prevState.cellVMs.filter(c => c.cell.id !== arg.payload.data.cellId)];
                 const nextOrPrev = index === arg.prevState.cellVMs.length - 1 ? index - 1 : index;
-                if (arg.prevState.selectedCellId === arg.payload.data.cellId || arg.prevState.focusedCellId === arg.payload.data.cellId) {
+                if (previousSelection.selectedCellId === arg.payload.data.cellId || previousSelection.focusedCellId === arg.payload.data.cellId) {
                     if (nextOrPrev >= 0) {
-                        newVMs[nextOrPrev] = { ...newVMs[nextOrPrev], selected: true, focused: arg.prevState.focusedCellId === arg.payload.data.cellId };
-                        newSelection = newVMs[nextOrPrev].cell.id;
-                        newFocused = newVMs[nextOrPrev].focused ? newVMs[nextOrPrev].cell.id : undefined;
+                        newVMs[nextOrPrev] = { ...newVMs[nextOrPrev], selected: true, focused: previousSelection.focusedCellId === arg.payload.data.cellId };
                     }
                 }
 
                 return {
                     ...arg.prevState,
                     cellVMs: newVMs,
-                    selectedCellId: newSelection,
-                    focusedCellId: newFocused,
                     undoStack: Helpers.pushStack(arg.prevState.undoStack, arg.prevState.cellVMs),
                     skipNextScroll: true
                 };