From 88f7a12245a185f61627d3ccde642ff5b4f055b8 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 4 Feb 2020 19:29:37 -0800 Subject: [PATCH 01/12] Fixes --- .../interactiveWindowTypes.ts | 3 +- .../interactive-ipynb/nativeEditor.ts | 8 +- .../redux/reducers/types.ts | 32 +++++--- .../native-editor/nativeCell.tsx | 6 +- .../native-editor/nativeEditor.tsx | 6 +- .../native-editor/redux/actions.ts | 79 ++++++++++++++----- .../native-editor/redux/reducers/creation.ts | 11 --- .../native-editor/redux/reducers/effects.ts | 10 +-- .../native-editor/redux/reducers/execution.ts | 17 ++-- .../react-common/monacoEditor.tsx | 2 +- 10 files changed, 108 insertions(+), 66 deletions(-) diff --git a/src/client/datascience/interactive-common/interactiveWindowTypes.ts b/src/client/datascience/interactive-common/interactiveWindowTypes.ts index d5772c5af964..76f9f3491d8e 100644 --- a/src/client/datascience/interactive-common/interactiveWindowTypes.ts +++ b/src/client/datascience/interactive-common/interactiveWindowTypes.ts @@ -3,7 +3,7 @@ 'use strict'; import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api'; import { IServerState } from '../../../datascience-ui/interactive-common/mainState'; -import { CommonActionType, IAddCellAction, ICellAction } from '../../../datascience-ui/interactive-common/redux/reducers/types'; +import { CommonActionType, IAddCellAction, ICellAction, ICellAndCursorAction } from '../../../datascience-ui/interactive-common/redux/reducers/types'; import { CssMessages, IGetCssRequest, IGetCssResponse, IGetMonacoThemeRequest, SharedMessages } from '../messages'; import { IGetMonacoThemeResponse } from '../monacoMessages'; import { ICell, IInteractiveWindowInfo, IJupyterVariable, IJupyterVariablesRequest, IJupyterVariablesResponse } from '../types'; @@ -377,6 +377,7 @@ export class IInteractiveWindowMapping { public [InteractiveWindowMessages.NotebookRunAllCells]: never | undefined; public [InteractiveWindowMessages.NotebookRunSelectedCell]: never | undefined; public [InteractiveWindowMessages.NotebookAddCellBelow]: IAddCellAction; + public [CommonActionType.FOCUS_CELL]: ICellAndCursorAction; public [InteractiveWindowMessages.DoSave]: never | undefined; public [InteractiveWindowMessages.ExecutionRendered]: IRenderComplete; public [InteractiveWindowMessages.FocusedCellEditor]: IFocusedCellEditor; diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index a1701d2dbe3b..7c295fb18841 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -7,7 +7,10 @@ import { inject, injectable, multiInject, named } from 'inversify'; import * as path from 'path'; import { Event, EventEmitter, Memento, Uri, ViewColumn, WebviewPanel } from 'vscode'; +import * as uuid from 'uuid/v4'; import { createCodeCell, createErrorOutput } from '../../../datascience-ui/common/cellFactory'; +import { CursorPos } from '../../../datascience-ui/interactive-common/mainState'; +import { CommonActionType } from '../../../datascience-ui/interactive-common/redux/reducers/types'; import { IApplicationShell, ICommandManager, IDocumentManager, ILiveShareApi, IWebPanelProvider, IWorkspaceService } from '../../common/application/types'; import { ContextKey } from '../../common/contextKey'; import { traceError } from '../../common/logger'; @@ -273,7 +276,10 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { } public addCellBelow() { - this.postMessage(InteractiveWindowMessages.NotebookAddCellBelow).ignoreErrors(); + const newCellId = uuid(); + this.postMessage(InteractiveWindowMessages.NotebookAddCellBelow, { newCellId }).ignoreErrors(); + // tslint:disable-next-line: no-any + this.postMessage(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Top } as any).ignoreErrors(); } public async removeAllCells(): Promise { diff --git a/src/datascience-ui/interactive-common/redux/reducers/types.ts b/src/datascience-ui/interactive-common/redux/reducers/types.ts index 58bfdf71ed3f..063242edf6f7 100644 --- a/src/datascience-ui/interactive-common/redux/reducers/types.ts +++ b/src/datascience-ui/interactive-common/redux/reducers/types.ts @@ -3,7 +3,7 @@ 'use strict'; import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api'; -import { InteractiveWindowMessages, IShowDataViewer, NativeCommandType } from '../../../../client/datascience/interactive-common/interactiveWindowTypes'; +import { IInteractiveWindowMapping, InteractiveWindowMessages, IShowDataViewer, NativeCommandType } from '../../../../client/datascience/interactive-common/interactiveWindowTypes'; import { BaseReduxActionPayload } from '../../../../client/datascience/interactive-common/types'; import { IJupyterVariablesRequest } from '../../../../client/datascience/types'; import { ActionWithPayload, ReducerArg } from '../../../react-common/reduxUtils'; @@ -71,7 +71,7 @@ export type CommonActionTypeMapping = { [CommonActionType.INSERT_BELOW]: ICellAction & IAddCellAction; [CommonActionType.INSERT_ABOVE_FIRST]: IAddCellAction; [CommonActionType.FOCUS_CELL]: ICellAndCursorAction; - [CommonActionType.UNFOCUS_CELL]: ICodeAction; + [CommonActionType.UNFOCUS_CELL]: ICellAction | ICodeAction; [CommonActionType.ADD_NEW_CELL]: IAddCellAction; [CommonActionType.EDIT_CELL]: IEditCellAction; [CommonActionType.EXECUTE_CELL]: IExecuteAction; @@ -145,14 +145,9 @@ export interface IEditCellAction extends ICodeAction { // I.e. when using the operation `add`, we need the corresponding `IAddCellAction`. // They are mutually exclusive, if not `add`, then there's no `newCellId`. -export type IExecuteAction = - | (ICodeAction & { - moveOp: 'select' | 'none'; - }) - | (ICodeAction & - IAddCellAction & { - moveOp: 'add'; - }); +export type IExecuteAction = ICodeAction & { + moveOp: 'select' | 'none' | 'add'; +}; export interface ICodeCreatedAction extends ICellAction { modelId: string; @@ -178,3 +173,20 @@ export interface IChangeCellTypeAction { currentCode: string; } export type CommonAction = ActionWithPayload; + +/* + Create a type which has `unknown` for all `CustomActionType` items that do not have a corresponding entry in `CommonActionTypeMapping`. + This provides the ability for strong typeing of actions created for a specific command type. + */ +type UnknownTypeForMissingMappings = CommonActionTypeMapping & { [W in CommonActionType]: unknown }; + +export function createIncomingActionWithPayload(type: K, data: V): CommonAction; +export function createIncomingActionWithPayload(type: K, data: P): CommonAction

; +// tslint:disable-next-line: no-any +export function createIncomingActionWithPayload(type: any, data: any): CommonAction { + // tslint:disable-next-line: no-any + return { type, payload: ({ data, messageDirection: 'incoming' } as any) as BaseReduxActionPayload } as any; +} +export function createIncomingAction(type: CommonActionType | InteractiveWindowMessages): CommonAction { + return { type, payload: { messageDirection: 'incoming', data: undefined } }; +} diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index c3158de886b9..e8c81c792d38 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -25,7 +25,7 @@ import { Image, ImageName } from '../react-common/image'; import { ImageButton } from '../react-common/imageButton'; import { getLocString } from '../react-common/locReactSide'; import { AddCellLine } from './addCellLine'; -import { actionCreators } from './redux/actions'; +import { ActionCreators, mapDispatchToProps } from './redux/actions'; interface INativeCellBaseProps { role?: string; @@ -44,7 +44,7 @@ interface INativeCellBaseProps { themeMatplotlibPlots: boolean | undefined; } -type INativeCellProps = INativeCellBaseProps & typeof actionCreators; +type INativeCellProps = INativeCellBaseProps & ActionCreators; // tslint:disable: react-this-binding-issue export class NativeCell extends React.Component { @@ -689,5 +689,5 @@ export class NativeCell extends React.Component { // Main export, return a redux connected editor export function getConnectedNativeCell() { - return connect(null, actionCreators)(NativeCell); + return connect(null, mapDispatchToProps)(NativeCell); } diff --git a/src/datascience-ui/native-editor/nativeEditor.tsx b/src/datascience-ui/native-editor/nativeEditor.tsx index e9b28641215d..bd8024e371ed 100644 --- a/src/datascience-ui/native-editor/nativeEditor.tsx +++ b/src/datascience-ui/native-editor/nativeEditor.tsx @@ -21,9 +21,9 @@ import { Progress } from '../react-common/progress'; import { AddCellLine } from './addCellLine'; import { getConnectedNativeCell } from './nativeCell'; import './nativeEditor.less'; -import { actionCreators } from './redux/actions'; +import { ActionCreators, mapDispatchToProps } from './redux/actions'; -type INativeEditorProps = IMainWithVariables & typeof actionCreators; +type INativeEditorProps = IMainWithVariables & ActionCreators; function mapStateToProps(state: IStore): IMainWithVariables { return { ...state.main, variableState: state.variables }; @@ -414,5 +414,5 @@ export class NativeEditor extends React.Component { // Main export, return a redux connected editor export function getConnectedNativeEditor() { - return connect(mapStateToProps, actionCreators)(NativeEditor); + return connect(mapStateToProps, mapDispatchToProps)(NativeEditor); } diff --git a/src/datascience-ui/native-editor/redux/actions.ts b/src/datascience-ui/native-editor/redux/actions.ts index 8619a0db3e50..571bbe95f01e 100644 --- a/src/datascience-ui/native-editor/redux/actions.ts +++ b/src/datascience-ui/native-editor/redux/actions.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. 'use strict'; import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api'; +import { bindActionCreators, Dispatch } from 'redux'; import * as uuid from 'uuid/v4'; import { InteractiveWindowMessages, NativeCommandType } from '../../../client/datascience/interactive-common/interactiveWindowTypes'; import { IJupyterVariable, IJupyterVariablesRequest } from '../../../client/datascience/types'; @@ -10,39 +11,71 @@ import { createIncomingAction, createIncomingActionWithPayload } from '../../int import { CommonAction, CommonActionType, - IAddCellAction, ICellAction, ICellAndCursorAction, - IChangeCellTypeAction, ICodeAction, ICodeCreatedAction, IEditCellAction, - IExecuteAction, ILinkClickAction, ISendCommandAction, IShowDataViewerAction } from '../../interactive-common/redux/reducers/types'; +const actionCreatorsWithDispatch = (dispatch: Dispatch) => { + return { + addCell: () => { + const newCellId = uuid(); + dispatch(createIncomingActionWithPayload(CommonActionType.ADD_NEW_CELL, { newCellId })); + dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current })); + }, + insertAboveFirst: () => { + const newCellId = uuid(); + dispatch(createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE_FIRST, { newCellId })); + dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current })); + }, + insertAbove: (cellId: string | undefined) => { + // Use settimeout to ensure the newly created cell does not end up with the key that was entered by user. + // E.g. if user types `a`, then we create a new cell, however old approach would result in the new editor having a character `a`. + setTimeout(() => { + const newCellId = uuid(); + dispatch(createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE, { cellId, newCellId })); + dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current })); + }, 1); + }, + insertBelow: (cellId: string | undefined) => { + if (!cellId) { + return; + } + // Use settimeout to ensure the newly created cell does not end up with the key that was entered by user. + // E.g. if user types `a`, then we create a new cell, however old approach would result in the new editor having a character `a`. + setTimeout(() => { + const newCellId = uuid(); + dispatch(createIncomingActionWithPayload(CommonActionType.INSERT_BELOW, { cellId, newCellId })); + dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current })); + }, 1); + }, + executeCell: (cellId: string, code: string, moveOp: 'add' | 'select' | 'none') => { + dispatch(createIncomingActionWithPayload(CommonActionType.EXECUTE_CELL, { cellId, code, moveOp })); + if (moveOp === 'add') { + const newCellId = uuid(); + dispatch(createIncomingActionWithPayload(CommonActionType.INSERT_BELOW, { cellId, newCellId })); + dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current })); + } + }, + changeCellType: (cellId: string, currentCode: string) => { + dispatch(createIncomingActionWithPayload(CommonActionType.CHANGE_CELL_TYPE, { cellId, currentCode })); + dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId, cursorPos: CursorPos.Current })); + } + }; +}; + // See https://react-redux.js.org/using-react-redux/connect-mapdispatch#defining-mapdispatchtoprops-as-an-object -export const actionCreators = { - insertAbove: (cellId: string | undefined): CommonAction => - createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE, { cellId, newCellId: uuid() }), - insertAboveFirst: (): CommonAction => createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE_FIRST, { newCellId: uuid() }), - insertBelow: (cellId: string | undefined): CommonAction => - createIncomingActionWithPayload(CommonActionType.INSERT_BELOW, { cellId, newCellId: uuid() }), +const actionCreators = { focusCell: (cellId: string, cursorPos: CursorPos = CursorPos.Current): CommonAction => createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId, cursorPos }), unfocusCell: (cellId: string, code: string): CommonAction => createIncomingActionWithPayload(CommonActionType.UNFOCUS_CELL, { cellId, code }), selectCell: (cellId: string, cursorPos: CursorPos = CursorPos.Current): CommonAction => createIncomingActionWithPayload(CommonActionType.SELECT_CELL, { cellId, cursorPos }), - addCell: (): CommonAction => createIncomingActionWithPayload(CommonActionType.ADD_NEW_CELL, { newCellId: uuid() }), - executeCell: (cellId: string, code: string, moveOp: 'add' | 'select' | 'none'): CommonAction => { - if (moveOp === 'add') { - return createIncomingActionWithPayload(CommonActionType.EXECUTE_CELL, { cellId, code, moveOp, newCellId: uuid() }); - } else { - return createIncomingActionWithPayload(CommonActionType.EXECUTE_CELL, { cellId, code, moveOp }); - } - }, executeAllCells: (): CommonAction => createIncomingAction(CommonActionType.EXECUTE_ALL_CELLS), executeAbove: (cellId: string): CommonAction => createIncomingActionWithPayload(CommonActionType.EXECUTE_ABOVE, { cellId }), executeCellAndBelow: (cellId: string, code: string): CommonAction => createIncomingActionWithPayload(CommonActionType.EXECUTE_CELL_AND_BELOW, { cellId, code }), @@ -58,8 +91,7 @@ export const actionCreators = { createIncomingActionWithPayload(CommonActionType.SEND_COMMAND, { command, commandType }), moveCellUp: (cellId: string): CommonAction => createIncomingActionWithPayload(CommonActionType.MOVE_CELL_UP, { cellId }), moveCellDown: (cellId: string): CommonAction => createIncomingActionWithPayload(CommonActionType.MOVE_CELL_DOWN, { cellId }), - changeCellType: (cellId: string, currentCode: string): CommonAction => - createIncomingActionWithPayload(CommonActionType.CHANGE_CELL_TYPE, { cellId, currentCode }), + // changeCellType: (cellId: string, currentCode: string) => createIncomingActionWithPayload(CommonActionType.CHANGE_CELL_TYPE, { cellId, currentCode }), toggleLineNumbers: (cellId: string): CommonAction => createIncomingActionWithPayload(CommonActionType.TOGGLE_LINE_NUMBERS, { cellId }), toggleOutput: (cellId: string): CommonAction => createIncomingActionWithPayload(CommonActionType.TOGGLE_OUTPUT, { cellId }), deleteCell: (cellId: string): CommonAction => createIncomingActionWithPayload(InteractiveWindowMessages.DeleteCell, { cellId }), @@ -82,3 +114,12 @@ export const actionCreators = { getVariableData: (newExecutionCount: number, startIndex: number = 0, pageSize: number = 100): CommonAction => createIncomingActionWithPayload(CommonActionType.GET_VARIABLE_DATA, { executionCount: newExecutionCount, sortColumn: 'name', sortAscending: true, startIndex, pageSize }) }; + +export type ActionCreators = typeof actionCreators & ReturnType; + +export function mapDispatchToProps(dispatch: Dispatch): ActionCreators { + return { + ...actionCreatorsWithDispatch(dispatch), + ...bindActionCreators(actionCreators, dispatch) + }; +} diff --git a/src/datascience-ui/native-editor/redux/reducers/creation.ts b/src/datascience-ui/native-editor/redux/reducers/creation.ts index 2aa41b26a987..3cfec3a00ed2 100644 --- a/src/datascience-ui/native-editor/redux/reducers/creation.ts +++ b/src/datascience-ui/native-editor/redux/reducers/creation.ts @@ -8,7 +8,6 @@ import { createCellVM, createEmptyCell, CursorPos, extractInputText, getSelected import { createPostableAction } from '../../../interactive-common/redux/helpers'; import { Helpers } from '../../../interactive-common/redux/reducers/helpers'; import { IAddCellAction, ICellAction } from '../../../interactive-common/redux/reducers/types'; -import { actionCreators } from '../actions'; import { NativeEditorReducerArg } from '../mapping'; export namespace Creation { @@ -60,11 +59,6 @@ export namespace Creation { createPostableAction(InteractiveWindowMessages.InsertCell, { cell: newVM.cell, index: position, code: '', codeCellAboveId: findFirstCodeCellAbove(newList, position) }) ); - // Queue up an action to set focus to the cell we're inserting - setTimeout(() => { - arg.queueAction(actionCreators.focusCell(newVM.cell.id)); - }); - return result; } @@ -95,11 +89,6 @@ export namespace Creation { createPostableAction(InteractiveWindowMessages.InsertCell, { cell: newVM.cell, index, code: '', codeCellAboveId: findFirstCodeCellAbove(newList, position) }) ); - // Queue up an action to set focus to the cell we're inserting - setTimeout(() => { - arg.queueAction(actionCreators.focusCell(newVM.cell.id)); - }); - return result; } diff --git a/src/datascience-ui/native-editor/redux/reducers/effects.ts b/src/datascience-ui/native-editor/redux/reducers/effects.ts index 6bfa022ef4c8..79be819af983 100644 --- a/src/datascience-ui/native-editor/redux/reducers/effects.ts +++ b/src/datascience-ui/native-editor/redux/reducers/effects.ts @@ -51,7 +51,7 @@ export namespace Effects { return arg.prevState; } - export function unfocusCell(arg: NativeEditorReducerArg): IMainState { + export function unfocusCell(arg: NativeEditorReducerArg): IMainState { // Unfocus the cell const index = arg.prevState.cellVMs.findIndex(c => c.cell.id === arg.payload.data.cellId); const selectionInfo = getSelectedAndFocusedInfo(arg.prevState); @@ -60,13 +60,13 @@ export namespace Effects { const current = arg.prevState.cellVMs[index]; const newCell = { ...current, - inputBlockText: arg.payload.data.code, + inputBlockText: 'code' in arg.payload.data ? arg.payload.data.code : current.inputBlockText, focused: false, cell: { ...current.cell, data: { ...current.cell.data, - source: arg.payload.data.code + source: 'code' in arg.payload.data ? arg.payload.data.code : current.cell.data.source } } }; @@ -84,12 +84,12 @@ export namespace Effects { const current = arg.prevState.cellVMs[index]; const newCell = { ...current, - inputBlockText: arg.payload.data.code, + inputBlockText: 'code' in arg.payload.data ? arg.payload.data.code : current.inputBlockText, cell: { ...current.cell, data: { ...current.cell.data, - source: arg.payload.data.code + source: 'code' in arg.payload.data ? arg.payload.data.code : current.cell.data.source } } }; diff --git a/src/datascience-ui/native-editor/redux/reducers/execution.ts b/src/datascience-ui/native-editor/redux/reducers/execution.ts index bfeb0ebbd176..76d17df1b570 100644 --- a/src/datascience-ui/native-editor/redux/reducers/execution.ts +++ b/src/datascience-ui/native-editor/redux/reducers/execution.ts @@ -14,7 +14,6 @@ import { Helpers } from '../../../interactive-common/redux/reducers/helpers'; import { CommonActionType, ICellAction, IChangeCellTypeAction, ICodeAction, IExecuteAction } from '../../../interactive-common/redux/reducers/types'; import { QueueAnotherFunc } from '../../../react-common/reduxUtils'; import { NativeEditorReducerArg } from '../mapping'; -import { Creation } from './creation'; import { Effects } from './effects'; export namespace Execution { @@ -69,11 +68,7 @@ export namespace Execution { const executeResult = executeRange(arg.prevState, index, index, [arg.payload.data.code], arg.queueAction); // Modify the execute result if moving - // Use `if` instead of `switch case` to ensure type safety. - if (arg.payload.data.moveOp === 'add') { - // Add a new cell below - return Creation.insertBelow({ ...arg, prevState: executeResult, payload: { ...arg.payload, data: { ...arg.payload.data } } }); - } else if (arg.payload.data.moveOp === 'select') { + if (arg.payload.data.moveOp === 'select') { // Select the cell below this one, but don't focus it if (index < arg.prevState.cellVMs.length - 1) { return Effects.selectCell( @@ -186,12 +181,10 @@ export namespace Execution { arg.queueAction(createPostableAction(InteractiveWindowMessages.RemoveCell, { id: current.cell.id })); } - // When changing a cell type, also give the cell focus. - return Effects.focusCell({ - ...arg, - prevState: { ...arg.prevState, cellVMs }, - payload: { ...arg.payload, data: { cellId: arg.payload.data.cellId, cursorPos: CursorPos.Current } } - }); + return { + ...arg.prevState, + cellVMs + }; } return arg.prevState; diff --git a/src/datascience-ui/react-common/monacoEditor.tsx b/src/datascience-ui/react-common/monacoEditor.tsx index b933362ce3e8..edd6094b2cb0 100644 --- a/src/datascience-ui/react-common/monacoEditor.tsx +++ b/src/datascience-ui/react-common/monacoEditor.tsx @@ -600,7 +600,7 @@ export class MonacoEditor extends React.Component { - if (!item.className) { + if (typeof item.className !== 'string') { return false; } // Check if user is hovering over a parameter widget. From 0b904af72c5e787fa36878f4e3f8a345fdd08544 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 5 Feb 2020 09:29:09 -0800 Subject: [PATCH 02/12] Remove cyclic messages --- src/datascience-ui/native-editor/redux/reducers/creation.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/datascience-ui/native-editor/redux/reducers/creation.ts b/src/datascience-ui/native-editor/redux/reducers/creation.ts index 3cfec3a00ed2..2e3f69a0e492 100644 --- a/src/datascience-ui/native-editor/redux/reducers/creation.ts +++ b/src/datascience-ui/native-editor/redux/reducers/creation.ts @@ -121,9 +121,6 @@ export namespace Creation { } export function deleteAllCells(arg: NativeEditorReducerArg): IMainState { - // Send messages to other side to indicate the deletes - arg.queueAction(createPostableAction(InteractiveWindowMessages.DeleteAllCells)); - // Just leave one single blank empty cell const newVM: ICellViewModel = { cell: createEmptyCell(arg.payload.data.newCellId, null), From 19b193483fd4c70f8200435781c39681a6742615 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 6 Feb 2020 13:52:08 -0800 Subject: [PATCH 03/12] Misc --- .../interactive-common/synchronization.ts | 2 ++ .../history-react/redux/actions.ts | 2 +- .../interactive-common/redux/helpers.ts | 6 +++--- .../redux/reducers/types.ts | 21 +++---------------- .../native-editor/redux/actions.ts | 4 ++-- 5 files changed, 11 insertions(+), 24 deletions(-) diff --git a/src/client/datascience/interactive-common/synchronization.ts b/src/client/datascience/interactive-common/synchronization.ts index 54c0decb7007..8316e31b4930 100644 --- a/src/client/datascience/interactive-common/synchronization.ts +++ b/src/client/datascience/interactive-common/synchronization.ts @@ -70,6 +70,8 @@ const messageWithMessageTypes: MessageMapping & Messa [CommonActionType.TOGGLE_VARIABLE_EXPLORER]: MessageType.syncWithLiveShare, [CommonActionType.UNFOCUS_CELL]: MessageType.syncWithLiveShare, [CommonActionType.UNMOUNT]: MessageType.other, + [CommonActionType.PostOutgoingMessage]: MessageType.other, + [CommonActionType.REFRESH_VARIABLES]: MessageType.other, // Types from InteractiveWindowMessages [InteractiveWindowMessages.Activate]: MessageType.other, diff --git a/src/datascience-ui/history-react/redux/actions.ts b/src/datascience-ui/history-react/redux/actions.ts index 905c40e733a2..7fcbe7be4363 100644 --- a/src/datascience-ui/history-react/redux/actions.ts +++ b/src/datascience-ui/history-react/redux/actions.ts @@ -27,7 +27,7 @@ export const actionCreators = { undo: (): CommonAction => createIncomingAction(InteractiveWindowMessages.Undo), redo: (): CommonAction => createIncomingAction(InteractiveWindowMessages.Redo), linkClick: (href: string): CommonAction => createIncomingActionWithPayload(CommonActionType.LINK_CLICK, { href }), - showPlot: (imageHtml: string): CommonAction => createIncomingActionWithPayload(InteractiveWindowMessages.ShowPlot, imageHtml), + showPlot: (imageHtml: string) => createIncomingActionWithPayload(InteractiveWindowMessages.ShowPlot, imageHtml), toggleInputBlock: (cellId: string): CommonAction => createIncomingActionWithPayload(CommonActionType.TOGGLE_INPUT_BLOCK, { cellId }), gotoCell: (cellId: string): CommonAction => createIncomingActionWithPayload(CommonActionType.GOTO_CELL, { cellId }), copyCellCode: (cellId: string): CommonAction => createIncomingActionWithPayload(CommonActionType.COPY_CELL_CODE, { cellId }), diff --git a/src/datascience-ui/interactive-common/redux/helpers.ts b/src/datascience-ui/interactive-common/redux/helpers.ts index 685b3c251d96..3717e514d350 100644 --- a/src/datascience-ui/interactive-common/redux/helpers.ts +++ b/src/datascience-ui/interactive-common/redux/helpers.ts @@ -7,7 +7,7 @@ import * as Redux from 'redux'; import { IInteractiveWindowMapping, InteractiveWindowMessages } from '../../../client/datascience/interactive-common/interactiveWindowTypes'; import { BaseReduxActionPayload } from '../../../client/datascience/interactive-common/types'; import { CssMessages, SharedMessages } from '../../../client/datascience/messages'; -import { CommonAction, CommonActionType } from './reducers/types'; +import { CommonAction, CommonActionType, CommonActionTypeMapping } from './reducers/types'; const AllowedMessages = [...Object.values(InteractiveWindowMessages), ...Object.values(CssMessages), ...Object.values(SharedMessages), ...Object.values(CommonActionType)]; export function isAllowedMessage(message: string) { @@ -18,9 +18,9 @@ export function isAllowedAction(action: Redux.AnyAction) { return isAllowedMessage(action.type); } -export function createIncomingActionWithPayload(type: CommonActionType | InteractiveWindowMessages, data: T): CommonAction { +export function createIncomingActionWithPayload(type: K, data: M[K]): CommonAction { // tslint:disable-next-line: no-any - return { type, payload: ({ data, messageDirection: 'incoming' } as any) as BaseReduxActionPayload }; + return { type, payload: { data, messageDirection: 'incoming' } as any } as any; } export function createIncomingAction(type: CommonActionType | InteractiveWindowMessages): CommonAction { return { type, payload: { messageDirection: 'incoming', data: undefined } }; diff --git a/src/datascience-ui/interactive-common/redux/reducers/types.ts b/src/datascience-ui/interactive-common/redux/reducers/types.ts index 063242edf6f7..edc83783e8b3 100644 --- a/src/datascience-ui/interactive-common/redux/reducers/types.ts +++ b/src/datascience-ui/interactive-common/redux/reducers/types.ts @@ -3,7 +3,7 @@ 'use strict'; import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api'; -import { IInteractiveWindowMapping, InteractiveWindowMessages, IShowDataViewer, NativeCommandType } from '../../../../client/datascience/interactive-common/interactiveWindowTypes'; +import { InteractiveWindowMessages, IShowDataViewer, NativeCommandType } from '../../../../client/datascience/interactive-common/interactiveWindowTypes'; import { BaseReduxActionPayload } from '../../../../client/datascience/interactive-common/types'; import { IJupyterVariablesRequest } from '../../../../client/datascience/types'; import { ActionWithPayload, ReducerArg } from '../../../react-common/reduxUtils'; @@ -107,6 +107,8 @@ export type CommonActionTypeMapping = { [CommonActionType.CODE_CREATED]: ICodeCreatedAction; [CommonActionType.GET_VARIABLE_DATA]: IJupyterVariablesRequest; [CommonActionType.TOGGLE_VARIABLE_EXPLORER]: never | undefined; + [CommonActionType.PostOutgoingMessage]: never | undefined; + [CommonActionType.REFRESH_VARIABLES]: never | undefined; }; export interface IShowDataViewerAction extends IShowDataViewer {} @@ -173,20 +175,3 @@ export interface IChangeCellTypeAction { currentCode: string; } export type CommonAction = ActionWithPayload; - -/* - Create a type which has `unknown` for all `CustomActionType` items that do not have a corresponding entry in `CommonActionTypeMapping`. - This provides the ability for strong typeing of actions created for a specific command type. - */ -type UnknownTypeForMissingMappings = CommonActionTypeMapping & { [W in CommonActionType]: unknown }; - -export function createIncomingActionWithPayload(type: K, data: V): CommonAction; -export function createIncomingActionWithPayload(type: K, data: P): CommonAction

; -// tslint:disable-next-line: no-any -export function createIncomingActionWithPayload(type: any, data: any): CommonAction { - // tslint:disable-next-line: no-any - return { type, payload: ({ data, messageDirection: 'incoming' } as any) as BaseReduxActionPayload } as any; -} -export function createIncomingAction(type: CommonActionType | InteractiveWindowMessages): CommonAction { - return { type, payload: { messageDirection: 'incoming', data: undefined } }; -} diff --git a/src/datascience-ui/native-editor/redux/actions.ts b/src/datascience-ui/native-editor/redux/actions.ts index 571bbe95f01e..6b26ae65c9b1 100644 --- a/src/datascience-ui/native-editor/redux/actions.ts +++ b/src/datascience-ui/native-editor/redux/actions.ts @@ -73,7 +73,7 @@ const actionCreatorsWithDispatch = (dispatch: Dispatch) => { const actionCreators = { focusCell: (cellId: string, cursorPos: CursorPos = CursorPos.Current): CommonAction => createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId, cursorPos }), - unfocusCell: (cellId: string, code: string): CommonAction => createIncomingActionWithPayload(CommonActionType.UNFOCUS_CELL, { cellId, code }), + unfocusCell: (cellId: string, code: string) => createIncomingActionWithPayload(CommonActionType.UNFOCUS_CELL, { cellId, code }), selectCell: (cellId: string, cursorPos: CursorPos = CursorPos.Current): CommonAction => createIncomingActionWithPayload(CommonActionType.SELECT_CELL, { cellId, cursorPos }), executeAllCells: (): CommonAction => createIncomingAction(CommonActionType.EXECUTE_ALL_CELLS), @@ -102,7 +102,7 @@ const actionCreators = { editCell: (cellId: string, changes: monacoEditor.editor.IModelContentChange[], modelId: string, code: string): CommonAction => createIncomingActionWithPayload(CommonActionType.EDIT_CELL, { cellId, changes, modelId, code }), linkClick: (href: string): CommonAction => createIncomingActionWithPayload(CommonActionType.LINK_CLICK, { href }), - showPlot: (imageHtml: string): CommonAction => createIncomingActionWithPayload(InteractiveWindowMessages.ShowPlot, imageHtml), + showPlot: (imageHtml: string) => createIncomingActionWithPayload(InteractiveWindowMessages.ShowPlot, imageHtml), gatherCell: (cellId: string | undefined): CommonAction => createIncomingActionWithPayload(CommonActionType.GATHER_CELL, { cellId }), editorLoaded: (): CommonAction => createIncomingAction(CommonActionType.EDITOR_LOADED), codeCreated: (cellId: string | undefined, modelId: string): CommonAction => From e43d169958cff5b6dcee0716ba2c0415332f7db8 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Feb 2020 15:18:12 -0800 Subject: [PATCH 04/12] Address code review comments --- .../interactive-common/synchronization.ts | 5 ++ .../redux/reducers/types.ts | 10 ++++ .../native-editor/nativeCell.tsx | 6 ++ .../native-editor/redux/actions.ts | 58 +++---------------- .../native-editor/redux/reducers/creation.ts | 28 ++++++++- .../native-editor/redux/reducers/execution.ts | 15 ++++- .../native-editor/redux/reducers/index.ts | 5 ++ 7 files changed, 74 insertions(+), 53 deletions(-) diff --git a/src/client/datascience/interactive-common/synchronization.ts b/src/client/datascience/interactive-common/synchronization.ts index 8316e31b4930..72eb94a4075b 100644 --- a/src/client/datascience/interactive-common/synchronization.ts +++ b/src/client/datascience/interactive-common/synchronization.ts @@ -30,6 +30,7 @@ export type IInteractiveActionMapping = MessageMapping & MessageMapping = { + [CommonActionType.ADD_AND_FOCUS_NEW_CELL]: MessageType.other, [CommonActionType.ADD_NEW_CELL]: MessageType.syncAcrossSameNotebooks | MessageType.syncWithLiveShare, [CommonActionType.ARROW_DOWN]: MessageType.syncWithLiveShare, [CommonActionType.ARROW_UP]: MessageType.syncWithLiveShare, @@ -39,6 +40,7 @@ const messageWithMessageTypes: MessageMapping & Messa [CommonActionType.COPY_CELL_CODE]: MessageType.other, [CommonActionType.EDITOR_LOADED]: MessageType.other, [CommonActionType.EDIT_CELL]: MessageType.syncAcrossSameNotebooks | MessageType.syncWithLiveShare, + [CommonActionType.EXECUTE_CELL_AND_ADVANCE]: MessageType.other, [CommonActionType.EXECUTE_ABOVE]: MessageType.other, [CommonActionType.EXECUTE_ALL_CELLS]: MessageType.other, [CommonActionType.EXECUTE_CELL]: MessageType.other, @@ -48,9 +50,12 @@ const messageWithMessageTypes: MessageMapping & Messa [CommonActionType.GATHER_CELL]: MessageType.other, [CommonActionType.GET_VARIABLE_DATA]: MessageType.other, [CommonActionType.GOTO_CELL]: MessageType.syncWithLiveShare, + [CommonActionType.INSERT_ABOVE_AND_FOCUS_NEW_CELL]: MessageType.other, [CommonActionType.INSERT_ABOVE]: MessageType.syncAcrossSameNotebooks | MessageType.syncWithLiveShare, + [CommonActionType.INSERT_ABOVE_FIRST_AND_FOCUS_NEW_CELL]: MessageType.other, [CommonActionType.INSERT_ABOVE_FIRST]: MessageType.syncAcrossSameNotebooks | MessageType.syncWithLiveShare, [CommonActionType.INSERT_BELOW]: MessageType.syncAcrossSameNotebooks | MessageType.syncWithLiveShare, + [CommonActionType.INSERT_BELOW_AND_FOCUS_NEW_CELL]: MessageType.other, [CommonActionType.INTERRUPT_KERNEL]: MessageType.other, [CommonActionType.LOADED_ALL_CELLS]: MessageType.other, [CommonActionType.LINK_CLICK]: MessageType.other, diff --git a/src/datascience-ui/interactive-common/redux/reducers/types.ts b/src/datascience-ui/interactive-common/redux/reducers/types.ts index edc83783e8b3..c4b4c677a221 100644 --- a/src/datascience-ui/interactive-common/redux/reducers/types.ts +++ b/src/datascience-ui/interactive-common/redux/reducers/types.ts @@ -22,6 +22,10 @@ import { CursorPos, IMainState } from '../../mainState'; */ export enum CommonActionType { + ADD_AND_FOCUS_NEW_CELL = 'action.add_new_cell_and_focus_cell', + INSERT_ABOVE_AND_FOCUS_NEW_CELL = 'action.insert_above_and_focus_cell', + INSERT_BELOW_AND_FOCUS_NEW_CELL = 'action.insert_below_and_focus_cell', + INSERT_ABOVE_FIRST_AND_FOCUS_NEW_CELL = 'action.insert_above_first_and_focus_cell', ADD_NEW_CELL = 'action.add_new_cell', ARROW_DOWN = 'action.arrow_down', ARROW_UP = 'action.arrow_up', @@ -34,6 +38,7 @@ export enum CommonActionType { EXECUTE_ABOVE = 'action.execute_above', EXECUTE_ALL_CELLS = 'action.execute_all_cells', EXECUTE_CELL = 'action.execute_cell', + EXECUTE_CELL_AND_ADVANCE = 'action.execute_cell_and_advance', EXECUTE_CELL_AND_BELOW = 'action.execute_cell_and_below', EXPORT = 'action.export', FOCUS_CELL = 'action.focus_cell', @@ -67,13 +72,18 @@ export enum CommonActionType { } export type CommonActionTypeMapping = { + [CommonActionType.ADD_AND_FOCUS_NEW_CELL]: IAddCellAction; [CommonActionType.INSERT_ABOVE]: ICellAction & IAddCellAction; [CommonActionType.INSERT_BELOW]: ICellAction & IAddCellAction; [CommonActionType.INSERT_ABOVE_FIRST]: IAddCellAction; + [CommonActionType.INSERT_ABOVE_FIRST_AND_FOCUS_NEW_CELL]: IAddCellAction; + [CommonActionType.INSERT_BELOW_AND_FOCUS_NEW_CELL]: ICellAction & IAddCellAction; + [CommonActionType.INSERT_ABOVE_AND_FOCUS_NEW_CELL]: ICellAction & IAddCellAction; [CommonActionType.FOCUS_CELL]: ICellAndCursorAction; [CommonActionType.UNFOCUS_CELL]: ICellAction | ICodeAction; [CommonActionType.ADD_NEW_CELL]: IAddCellAction; [CommonActionType.EDIT_CELL]: IEditCellAction; + [CommonActionType.EXECUTE_CELL_AND_ADVANCE]: IExecuteAction; [CommonActionType.EXECUTE_CELL]: IExecuteAction; [CommonActionType.EXECUTE_ALL_CELLS]: never | undefined; [CommonActionType.EXECUTE_ABOVE]: ICellAction; diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index e8c81c792d38..95c9a6804767 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -269,6 +269,7 @@ export class NativeCell extends React.Component { case 'l': if (!this.isFocused() && this.isSelected()) { e.stopPropagation(); + e.preventDefault(); this.props.toggleLineNumbers(cellId); this.props.sendCommand(NativeCommandType.ToggleLineNumbers, 'keyboard'); } @@ -276,6 +277,7 @@ export class NativeCell extends React.Component { case 'o': if (!this.isFocused() && this.isSelected()) { e.stopPropagation(); + e.preventDefault(); this.props.toggleOutput(cellId); this.props.sendCommand(NativeCommandType.ToggleOutput, 'keyboard'); } @@ -302,6 +304,7 @@ export class NativeCell extends React.Component { case 'a': if (!this.isFocused()) { e.stopPropagation(); + e.preventDefault(); this.props.insertAbove(cellId); this.props.sendCommand(NativeCommandType.InsertAbove, 'keyboard'); } @@ -309,6 +312,7 @@ export class NativeCell extends React.Component { case 'b': if (!this.isFocused()) { e.stopPropagation(); + e.preventDefault(); this.props.insertBelow(cellId); this.props.sendCommand(NativeCommandType.InsertBelow, 'keyboard'); } @@ -318,10 +322,12 @@ export class NativeCell extends React.Component { if (!this.isFocused()) { if (e.shiftKey && !e.ctrlKey && !e.altKey) { e.stopPropagation(); + e.preventDefault(); this.props.redo(); this.props.sendCommand(NativeCommandType.Redo, 'keyboard'); } else if (!e.shiftKey && !e.altKey && !e.ctrlKey) { e.stopPropagation(); + e.preventDefault(); this.props.undo(); this.props.sendCommand(NativeCommandType.Undo, 'keyboard'); } diff --git a/src/datascience-ui/native-editor/redux/actions.ts b/src/datascience-ui/native-editor/redux/actions.ts index 6b26ae65c9b1..06e717ae3463 100644 --- a/src/datascience-ui/native-editor/redux/actions.ts +++ b/src/datascience-ui/native-editor/redux/actions.ts @@ -21,59 +21,18 @@ import { IShowDataViewerAction } from '../../interactive-common/redux/reducers/types'; -const actionCreatorsWithDispatch = (dispatch: Dispatch) => { - return { - addCell: () => { - const newCellId = uuid(); - dispatch(createIncomingActionWithPayload(CommonActionType.ADD_NEW_CELL, { newCellId })); - dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current })); - }, - insertAboveFirst: () => { - const newCellId = uuid(); - dispatch(createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE_FIRST, { newCellId })); - dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current })); - }, - insertAbove: (cellId: string | undefined) => { - // Use settimeout to ensure the newly created cell does not end up with the key that was entered by user. - // E.g. if user types `a`, then we create a new cell, however old approach would result in the new editor having a character `a`. - setTimeout(() => { - const newCellId = uuid(); - dispatch(createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE, { cellId, newCellId })); - dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current })); - }, 1); - }, - insertBelow: (cellId: string | undefined) => { - if (!cellId) { - return; - } - // Use settimeout to ensure the newly created cell does not end up with the key that was entered by user. - // E.g. if user types `a`, then we create a new cell, however old approach would result in the new editor having a character `a`. - setTimeout(() => { - const newCellId = uuid(); - dispatch(createIncomingActionWithPayload(CommonActionType.INSERT_BELOW, { cellId, newCellId })); - dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current })); - }, 1); - }, - executeCell: (cellId: string, code: string, moveOp: 'add' | 'select' | 'none') => { - dispatch(createIncomingActionWithPayload(CommonActionType.EXECUTE_CELL, { cellId, code, moveOp })); - if (moveOp === 'add') { - const newCellId = uuid(); - dispatch(createIncomingActionWithPayload(CommonActionType.INSERT_BELOW, { cellId, newCellId })); - dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current })); - } - }, - changeCellType: (cellId: string, currentCode: string) => { - dispatch(createIncomingActionWithPayload(CommonActionType.CHANGE_CELL_TYPE, { cellId, currentCode })); - dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId, cursorPos: CursorPos.Current })); - } - }; -}; - // See https://react-redux.js.org/using-react-redux/connect-mapdispatch#defining-mapdispatchtoprops-as-an-object const actionCreators = { + addCell: () => createIncomingActionWithPayload(CommonActionType.ADD_AND_FOCUS_NEW_CELL, { newCellId: uuid() }), + insertAboveFirst: () => createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE_FIRST_AND_FOCUS_NEW_CELL, { newCellId: uuid() }), + insertAbove: (cellId: string | undefined) => createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE_AND_FOCUS_NEW_CELL, { cellId, newCellId: uuid() }), + insertBelow: (cellId: string | undefined) => createIncomingActionWithPayload(CommonActionType.INSERT_BELOW_AND_FOCUS_NEW_CELL, { cellId, newCellId: uuid() }), + executeCell: (cellId: string, code: string, moveOp: 'add' | 'select' | 'none') => + createIncomingActionWithPayload(CommonActionType.EXECUTE_CELL_AND_ADVANCE, { cellId, code, moveOp }), focusCell: (cellId: string, cursorPos: CursorPos = CursorPos.Current): CommonAction => createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId, cursorPos }), unfocusCell: (cellId: string, code: string) => createIncomingActionWithPayload(CommonActionType.UNFOCUS_CELL, { cellId, code }), + changeCellType: (cellId: string, currentCode: string) => createIncomingActionWithPayload(CommonActionType.CHANGE_CELL_TYPE, { cellId, currentCode }), selectCell: (cellId: string, cursorPos: CursorPos = CursorPos.Current): CommonAction => createIncomingActionWithPayload(CommonActionType.SELECT_CELL, { cellId, cursorPos }), executeAllCells: (): CommonAction => createIncomingAction(CommonActionType.EXECUTE_ALL_CELLS), @@ -115,11 +74,10 @@ const actionCreators = { createIncomingActionWithPayload(CommonActionType.GET_VARIABLE_DATA, { executionCount: newExecutionCount, sortColumn: 'name', sortAscending: true, startIndex, pageSize }) }; -export type ActionCreators = typeof actionCreators & ReturnType; +export type ActionCreators = typeof actionCreators; export function mapDispatchToProps(dispatch: Dispatch): ActionCreators { return { - ...actionCreatorsWithDispatch(dispatch), ...bindActionCreators(actionCreators, dispatch) }; } diff --git a/src/datascience-ui/native-editor/redux/reducers/creation.ts b/src/datascience-ui/native-editor/redux/reducers/creation.ts index 2e3f69a0e492..cf566895a0b0 100644 --- a/src/datascience-ui/native-editor/redux/reducers/creation.ts +++ b/src/datascience-ui/native-editor/redux/reducers/creation.ts @@ -5,9 +5,9 @@ import { ILoadAllCells, InteractiveWindowMessages } from '../../../../client/datascience/interactive-common/interactiveWindowTypes'; import { ICell, IDataScienceExtraSettings } from '../../../../client/datascience/types'; import { createCellVM, createEmptyCell, CursorPos, extractInputText, getSelectedAndFocusedInfo, ICellViewModel, IMainState } from '../../../interactive-common/mainState'; -import { createPostableAction } from '../../../interactive-common/redux/helpers'; +import { createIncomingActionWithPayload, createPostableAction } from '../../../interactive-common/redux/helpers'; import { Helpers } from '../../../interactive-common/redux/reducers/helpers'; -import { IAddCellAction, ICellAction } from '../../../interactive-common/redux/reducers/types'; +import { CommonActionType, IAddCellAction, ICellAction } from '../../../interactive-common/redux/reducers/types'; import { NativeEditorReducerArg } from '../mapping'; export namespace Creation { @@ -35,6 +35,30 @@ export namespace Creation { } } + export function addAndFocusCell(arg: NativeEditorReducerArg): IMainState { + arg.queueAction(createIncomingActionWithPayload(CommonActionType.ADD_NEW_CELL, { newCellId: arg.payload.data.newCellId })); + arg.queueAction(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: arg.payload.data.newCellId, cursorPos: CursorPos.Current })); + return arg.prevState; + } + + export function insertAboveAndFocusCell(arg: NativeEditorReducerArg): IMainState { + arg.queueAction(createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE, { cellId: arg.payload.data.cellId, newCellId: arg.payload.data.newCellId })); + arg.queueAction(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: arg.payload.data.newCellId, cursorPos: CursorPos.Current })); + return arg.prevState; + } + + export function insertBelowAndFocusCell(arg: NativeEditorReducerArg): IMainState { + arg.queueAction(createIncomingActionWithPayload(CommonActionType.INSERT_BELOW, { cellId: arg.payload.data.cellId, newCellId: arg.payload.data.newCellId })); + arg.queueAction(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: arg.payload.data.newCellId, cursorPos: CursorPos.Current })); + return arg.prevState; + } + + export function insertAboveFirstAndFocusCell(arg: NativeEditorReducerArg): IMainState { + arg.queueAction(createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE_FIRST, { newCellId: arg.payload.data.newCellId })); + arg.queueAction(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: arg.payload.data.newCellId, cursorPos: CursorPos.Current })); + return arg.prevState; + } + export function insertAbove(arg: NativeEditorReducerArg): IMainState { const newVM = prepareCellVM(createEmptyCell(arg.payload.data.newCellId, null), false, arg.prevState.settings); const newList = [...arg.prevState.cellVMs]; diff --git a/src/datascience-ui/native-editor/redux/reducers/execution.ts b/src/datascience-ui/native-editor/redux/reducers/execution.ts index 76d17df1b570..3c4d53969643 100644 --- a/src/datascience-ui/native-editor/redux/reducers/execution.ts +++ b/src/datascience-ui/native-editor/redux/reducers/execution.ts @@ -3,13 +3,14 @@ 'use strict'; // tslint:disable-next-line: no-require-imports no-var-requires const cloneDeep = require('lodash/cloneDeep'); +import * as uuid from 'uuid/v4'; import { CellMatcher } from '../../../../client/datascience/cellMatcher'; import { InteractiveWindowMessages } from '../../../../client/datascience/interactive-common/interactiveWindowTypes'; import { CellState } from '../../../../client/datascience/types'; import { concatMultilineStringInput } from '../../../common'; import { createCellFrom } from '../../../common/cellFactory'; import { CursorPos, getSelectedAndFocusedInfo, ICellViewModel, IMainState } from '../../../interactive-common/mainState'; -import { createPostableAction } from '../../../interactive-common/redux/helpers'; +import { createIncomingActionWithPayload, createPostableAction } from '../../../interactive-common/redux/helpers'; import { Helpers } from '../../../interactive-common/redux/reducers/helpers'; import { CommonActionType, ICellAction, IChangeCellTypeAction, ICodeAction, IExecuteAction } from '../../../interactive-common/redux/reducers/types'; import { QueueAnotherFunc } from '../../../react-common/reduxUtils'; @@ -61,6 +62,18 @@ export namespace Execution { return arg.prevState; } + export function executeCellAndAdvance(arg: NativeEditorReducerArg): IMainState { + arg.queueAction( + createIncomingActionWithPayload(CommonActionType.EXECUTE_CELL, { cellId: arg.payload.data.cellId, code: arg.payload.data.code, moveOp: arg.payload.data.moveOp }) + ); + if (arg.payload.data.moveOp === 'add') { + const newCellId = uuid(); + arg.queueAction(createIncomingActionWithPayload(CommonActionType.INSERT_BELOW, { cellId: arg.payload.data.cellId, newCellId })); + arg.queueAction(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current })); + } + return arg.prevState; + } + export function executeCell(arg: NativeEditorReducerArg): IMainState { const index = arg.prevState.cellVMs.findIndex(c => c.cell.id === arg.payload.data.cellId); if (index >= 0) { diff --git a/src/datascience-ui/native-editor/redux/reducers/index.ts b/src/datascience-ui/native-editor/redux/reducers/index.ts index ce91ddd0101e..49bcea7f9a15 100644 --- a/src/datascience-ui/native-editor/redux/reducers/index.ts +++ b/src/datascience-ui/native-editor/redux/reducers/index.ts @@ -16,12 +16,17 @@ import { Movement } from './movement'; // The list of reducers. 1 per message/action. export const reducerMap: Partial = { // State updates + [CommonActionType.INSERT_ABOVE_AND_FOCUS_NEW_CELL]: Creation.insertAboveAndFocusCell, + [CommonActionType.INSERT_ABOVE_FIRST_AND_FOCUS_NEW_CELL]: Creation.insertAboveFirstAndFocusCell, + [CommonActionType.INSERT_BELOW_AND_FOCUS_NEW_CELL]: Creation.insertBelowAndFocusCell, [CommonActionType.INSERT_ABOVE]: Creation.insertAbove, [CommonActionType.INSERT_ABOVE_FIRST]: Creation.insertAboveFirst, [CommonActionType.INSERT_BELOW]: Creation.insertBelow, [CommonActionType.FOCUS_CELL]: Effects.focusCell, [CommonActionType.UNFOCUS_CELL]: Effects.unfocusCell, + [CommonActionType.ADD_AND_FOCUS_NEW_CELL]: Creation.addAndFocusCell, [CommonActionType.ADD_NEW_CELL]: Creation.addNewCell, + [CommonActionType.EXECUTE_CELL_AND_ADVANCE]: Execution.executeCellAndAdvance, [CommonActionType.EXECUTE_CELL]: Execution.executeCell, [CommonActionType.EXECUTE_ALL_CELLS]: Execution.executeAllCells, [CommonActionType.EXECUTE_ABOVE]: Execution.executeAbove, From 5b9b0d6f2ab567338c363c769f60a8040bfe67e4 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Feb 2020 15:40:43 -0800 Subject: [PATCH 05/12] Oops --- .../native-editor/nativeCell.tsx | 20 +------------------ .../native-editor/nativeEditor.tsx | 3 +-- .../native-editor/redux/actions.ts | 14 ++----------- 3 files changed, 4 insertions(+), 33 deletions(-) diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index 43f650bac436..7c5bc4581a04 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -26,7 +26,6 @@ import { ImageButton } from '../react-common/imageButton'; import { getLocString } from '../react-common/locReactSide'; import { IMonacoModelContentChangeEvent } from '../react-common/monacoHelpers'; import { AddCellLine } from './addCellLine'; -import { ActionCreators, mapDispatchToProps } from './redux/actions'; interface INativeCellBaseProps { role?: string; @@ -46,7 +45,7 @@ interface INativeCellBaseProps { focusPending: number; } -type INativeCellProps = INativeCellBaseProps & ActionCreators; +type INativeCellProps = INativeCellBaseProps & typeof actionCreators; // tslint:disable: react-this-binding-issue export class NativeCell extends React.Component { @@ -320,23 +319,6 @@ export class NativeCell extends React.Component { this.props.sendCommand(NativeCommandType.InsertBelow, 'keyboard'); } break; - case 'z': - case 'Z': - if (!this.isFocused()) { - if (e.shiftKey && !e.ctrlKey && !e.altKey) { - e.stopPropagation(); - e.preventDefault(); - this.props.redo(); - this.props.sendCommand(NativeCommandType.Redo, 'keyboard'); - } else if (!e.shiftKey && !e.altKey && !e.ctrlKey) { - e.stopPropagation(); - e.preventDefault(); - this.props.undo(); - this.props.sendCommand(NativeCommandType.Undo, 'keyboard'); - } - } - break; - default: break; } diff --git a/src/datascience-ui/native-editor/nativeEditor.tsx b/src/datascience-ui/native-editor/nativeEditor.tsx index 960d757cf3d3..6a3e1156d5ac 100644 --- a/src/datascience-ui/native-editor/nativeEditor.tsx +++ b/src/datascience-ui/native-editor/nativeEditor.tsx @@ -21,9 +21,8 @@ import { Progress } from '../react-common/progress'; import { AddCellLine } from './addCellLine'; import { getConnectedNativeCell } from './nativeCell'; import './nativeEditor.less'; -import { ActionCreators, mapDispatchToProps } from './redux/actions'; -type INativeEditorProps = IMainWithVariables & ActionCreators; +type INativeEditorProps = IMainWithVariables & typeof actionCreators; function mapStateToProps(state: IStore): IMainWithVariables { return { ...state.main, variableState: state.variables }; diff --git a/src/datascience-ui/native-editor/redux/actions.ts b/src/datascience-ui/native-editor/redux/actions.ts index 4b4286beeb20..3ec7682b9e1c 100644 --- a/src/datascience-ui/native-editor/redux/actions.ts +++ b/src/datascience-ui/native-editor/redux/actions.ts @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. 'use strict'; -import { bindActionCreators, Dispatch } from 'redux'; import * as uuid from 'uuid/v4'; import { InteractiveWindowMessages, NativeCommandType } from '../../../client/datascience/interactive-common/interactiveWindowTypes'; import { IJupyterVariable, IJupyterVariablesRequest } from '../../../client/datascience/types'; @@ -22,7 +21,7 @@ import { import { IMonacoModelContentChangeEvent } from '../../react-common/monacoHelpers'; // See https://react-redux.js.org/using-react-redux/connect-mapdispatch#defining-mapdispatchtoprops-as-an-object -const actionCreators = { +export const actionCreators = { addCell: () => createIncomingActionWithPayload(CommonActionType.ADD_AND_FOCUS_NEW_CELL, { newCellId: uuid() }), insertAboveFirst: () => createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE_FIRST_AND_FOCUS_NEW_CELL, { newCellId: uuid() }), insertAbove: (cellId: string | undefined) => createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE_AND_FOCUS_NEW_CELL, { cellId, newCellId: uuid() }), @@ -32,7 +31,6 @@ const actionCreators = { focusCell: (cellId: string, cursorPos: CursorPos = CursorPos.Current): CommonAction => createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId, cursorPos }), unfocusCell: (cellId: string, code: string) => createIncomingActionWithPayload(CommonActionType.UNFOCUS_CELL, { cellId, code }), - changeCellType: (cellId: string, currentCode: string) => createIncomingActionWithPayload(CommonActionType.CHANGE_CELL_TYPE, { cellId, currentCode }), selectCell: (cellId: string, cursorPos: CursorPos = CursorPos.Current): CommonAction => createIncomingActionWithPayload(CommonActionType.SELECT_CELL, { cellId, cursorPos }), executeAllCells: (): CommonAction => createIncomingAction(CommonActionType.EXECUTE_ALL_CELLS), @@ -50,7 +48,7 @@ const actionCreators = { createIncomingActionWithPayload(CommonActionType.SEND_COMMAND, { command, commandType }), moveCellUp: (cellId: string): CommonAction => createIncomingActionWithPayload(CommonActionType.MOVE_CELL_UP, { cellId }), moveCellDown: (cellId: string): CommonAction => createIncomingActionWithPayload(CommonActionType.MOVE_CELL_DOWN, { cellId }), - // changeCellType: (cellId: string, currentCode: string) => createIncomingActionWithPayload(CommonActionType.CHANGE_CELL_TYPE, { cellId, currentCode }), + changeCellType: (cellId: string, currentCode: string) => createIncomingActionWithPayload(CommonActionType.CHANGE_CELL_TYPE, { cellId, currentCode }), toggleLineNumbers: (cellId: string): CommonAction => createIncomingActionWithPayload(CommonActionType.TOGGLE_LINE_NUMBERS, { cellId }), toggleOutput: (cellId: string): CommonAction => createIncomingActionWithPayload(CommonActionType.TOGGLE_OUTPUT, { cellId }), deleteCell: (cellId: string): CommonAction => createIncomingActionWithPayload(CommonActionType.DELETE_CELL, { cellId }), @@ -81,11 +79,3 @@ const actionCreators = { getVariableData: (newExecutionCount: number, startIndex: number = 0, pageSize: number = 100): CommonAction => createIncomingActionWithPayload(CommonActionType.GET_VARIABLE_DATA, { executionCount: newExecutionCount, sortColumn: 'name', sortAscending: true, startIndex, pageSize }) }; - -export type ActionCreators = typeof actionCreators; - -export function mapDispatchToProps(dispatch: Dispatch): ActionCreators { - return { - ...bindActionCreators(actionCreators, dispatch) - }; -} From b27a966b985f1a1941585f24ca64f5a6d94b254c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Feb 2020 15:47:12 -0800 Subject: [PATCH 06/12] More oops --- src/datascience-ui/native-editor/nativeCell.tsx | 3 ++- src/datascience-ui/native-editor/nativeEditor.tsx | 3 ++- src/datascience-ui/native-editor/redux/reducers/creation.ts | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index 7c5bc4581a04..f3868aea96ab 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -26,6 +26,7 @@ import { ImageButton } from '../react-common/imageButton'; import { getLocString } from '../react-common/locReactSide'; import { IMonacoModelContentChangeEvent } from '../react-common/monacoHelpers'; import { AddCellLine } from './addCellLine'; +import { actionCreators } from './redux/actions'; interface INativeCellBaseProps { role?: string; @@ -683,5 +684,5 @@ export class NativeCell extends React.Component { // Main export, return a redux connected editor export function getConnectedNativeCell() { - return connect(null, mapDispatchToProps)(NativeCell); + return connect(null, actionCreators)(NativeCell); } diff --git a/src/datascience-ui/native-editor/nativeEditor.tsx b/src/datascience-ui/native-editor/nativeEditor.tsx index 6a3e1156d5ac..f5b089bf8869 100644 --- a/src/datascience-ui/native-editor/nativeEditor.tsx +++ b/src/datascience-ui/native-editor/nativeEditor.tsx @@ -21,6 +21,7 @@ import { Progress } from '../react-common/progress'; import { AddCellLine } from './addCellLine'; import { getConnectedNativeCell } from './nativeCell'; import './nativeEditor.less'; +import { actionCreators } from './redux/actions'; type INativeEditorProps = IMainWithVariables & typeof actionCreators; @@ -400,5 +401,5 @@ export class NativeEditor extends React.Component { // Main export, return a redux connected editor export function getConnectedNativeEditor() { - return connect(mapStateToProps, mapDispatchToProps)(NativeEditor); + return connect(mapStateToProps, actionCreators)(NativeEditor); } diff --git a/src/datascience-ui/native-editor/redux/reducers/creation.ts b/src/datascience-ui/native-editor/redux/reducers/creation.ts index b02a11381aed..ec6f9d7c1229 100644 --- a/src/datascience-ui/native-editor/redux/reducers/creation.ts +++ b/src/datascience-ui/native-editor/redux/reducers/creation.ts @@ -11,6 +11,7 @@ import { Helpers } from '../../../interactive-common/redux/reducers/helpers'; import { Transfer } from '../../../interactive-common/redux/reducers/transfer'; import { CommonActionType, IAddCellAction, ICellAction } from '../../../interactive-common/redux/reducers/types'; import { NativeEditorReducerArg } from '../mapping'; +import { Effects } from './effects'; import { Execution } from './execution'; import { Movement } from './movement'; From 0fb47d39c570fe61370dd20418db54f3521fd040 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Feb 2020 16:18:34 -0800 Subject: [PATCH 07/12] More fixes --- .../native-editor/nativeCell.tsx | 6 ++-- .../native-editor/nativeEditor.tsx | 10 +++++-- .../native-editor/redux/reducers/movement.ts | 28 +++---------------- 3 files changed, 14 insertions(+), 30 deletions(-) diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index f3868aea96ab..7d66f1559b02 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -308,7 +308,7 @@ export class NativeCell extends React.Component { if (!this.isFocused()) { e.stopPropagation(); e.preventDefault(); - this.props.insertAbove(cellId); + setTimeout(() => this.props.insertAbove(cellId), 1); this.props.sendCommand(NativeCommandType.InsertAbove, 'keyboard'); } break; @@ -316,7 +316,7 @@ export class NativeCell extends React.Component { if (!this.isFocused()) { e.stopPropagation(); e.preventDefault(); - this.props.insertBelow(cellId); + setTimeout(() => this.props.insertBelow(cellId), 1); this.props.sendCommand(NativeCommandType.InsertBelow, 'keyboard'); } break; @@ -423,7 +423,7 @@ export class NativeCell extends React.Component { }; private addNewCell = () => { - this.props.insertBelow(this.cellId); + setTimeout(() => this.props.insertBelow(this.cellId), 1); this.props.sendCommand(NativeCommandType.AddToEnd, 'mouse'); }; diff --git a/src/datascience-ui/native-editor/nativeEditor.tsx b/src/datascience-ui/native-editor/nativeEditor.tsx index f5b089bf8869..172d2bfe84bb 100644 --- a/src/datascience-ui/native-editor/nativeEditor.tsx +++ b/src/datascience-ui/native-editor/nativeEditor.tsx @@ -37,6 +37,7 @@ export class NativeEditor extends React.Component { constructor(props: INativeEditorProps) { super(props); + this.insertAboveFirst = this.insertAboveFirst.bind(this); } public componentDidMount() { @@ -83,7 +84,7 @@ export class NativeEditor extends React.Component { const progressBar = this.props.busy && !this.props.testMode ? : undefined; const addCellLine = this.props.cellVMs.length === 0 ? null : ( - + ); return ( @@ -106,13 +107,16 @@ export class NativeEditor extends React.Component { ); } + private insertAboveFirst() { + setTimeout(() => this.props.insertAboveFirst(), 1); + } // tslint:disable: react-this-binding-issue // tslint:disable-next-line: max-func-body-length private renderToolbarPanel() { const selectedInfo = getSelectedAndFocusedInfo(this.props); const addCell = () => { - this.props.addCell(); + setTimeout(() => this.props.addCell(), 1); this.props.sendCommand(NativeCommandType.AddToEnd, 'mouse'); }; const runAll = () => { @@ -357,7 +361,7 @@ export class NativeEditor extends React.Component { } const addNewCell = () => { - this.props.insertBelow(cellVM.cell.id); + setTimeout(() => this.props.insertBelow(cellVM.cell.id), 1); this.props.sendCommand(NativeCommandType.AddToEnd, 'mouse'); }; const firstLine = index === 0; diff --git a/src/datascience-ui/native-editor/redux/reducers/movement.ts b/src/datascience-ui/native-editor/redux/reducers/movement.ts index 94492d2faf1d..ccb1323905b1 100644 --- a/src/datascience-ui/native-editor/redux/reducers/movement.ts +++ b/src/datascience-ui/native-editor/redux/reducers/movement.ts @@ -2,11 +2,11 @@ // Licensed under the MIT License. 'use strict'; import { CursorPos, IMainState } from '../../../interactive-common/mainState'; +import { createIncomingActionWithPayload } from '../../../interactive-common/redux/helpers'; import { Helpers } from '../../../interactive-common/redux/reducers/helpers'; import { Transfer } from '../../../interactive-common/redux/reducers/transfer'; -import { ICellAction, ICodeAction } from '../../../interactive-common/redux/reducers/types'; +import { CommonActionType, ICellAction, ICodeAction } from '../../../interactive-common/redux/reducers/types'; import { NativeEditorReducerArg } from '../mapping'; -import { Effects } from './effects'; export namespace Movement { export function swapCells(arg: NativeEditorReducerArg<{ firstCellId: string; secondCellId: string }>) { @@ -50,17 +50,7 @@ export namespace Movement { export function arrowUp(arg: NativeEditorReducerArg): IMainState { const index = arg.prevState.cellVMs.findIndex(c => c.cell.id === arg.payload.data.cellId); if (index > 0) { - const newState = Effects.selectCell({ ...arg, payload: { ...arg.payload, data: { cellId: arg.prevState.cellVMs[index - 1].cell.id, cursorPos: CursorPos.Bottom } } }); - const newVMs = [...newState.cellVMs]; - newVMs[index] = Helpers.asCellViewModel({ - ...newVMs[index], - inputBlockText: arg.payload.data.code, - cell: { ...newVMs[index].cell, data: { ...newVMs[index].cell.data, source: arg.payload.data.code } } - }); - return { - ...newState, - cellVMs: newVMs - }; + arg.queueAction(createIncomingActionWithPayload(CommonActionType.SELECT_CELL, { cellId: arg.prevState.cellVMs[index - 1].cell.id, cursorPos: CursorPos.Bottom })); } return arg.prevState; @@ -69,17 +59,7 @@ export namespace Movement { export function arrowDown(arg: NativeEditorReducerArg): IMainState { const index = arg.prevState.cellVMs.findIndex(c => c.cell.id === arg.payload.data.cellId); if (index < arg.prevState.cellVMs.length - 1) { - const newState = Effects.selectCell({ ...arg, payload: { ...arg.payload, data: { cellId: arg.prevState.cellVMs[index + 1].cell.id, cursorPos: CursorPos.Top } } }); - const newVMs = [...newState.cellVMs]; - newVMs[index] = Helpers.asCellViewModel({ - ...newVMs[index], - inputBlockText: arg.payload.data.code, - cell: { ...newVMs[index].cell, data: { ...newVMs[index].cell.data, source: arg.payload.data.code } } - }); - return { - ...newState, - cellVMs: newVMs - }; + arg.queueAction(createIncomingActionWithPayload(CommonActionType.SELECT_CELL, { cellId: arg.prevState.cellVMs[index + 1].cell.id, cursorPos: CursorPos.Bottom })); } return arg.prevState; From 909d4694651d3265ed465759981834b02e4800f3 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Feb 2020 17:08:03 -0800 Subject: [PATCH 08/12] Address code review comments --- src/client/datascience/interactive-ipynb/nativeEditor.ts | 7 +------ src/datascience-ui/native-editor/redux/reducers/index.ts | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index af374be2f6b2..a88148b9662f 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -9,8 +9,6 @@ import { Event, EventEmitter, Memento, Uri, ViewColumn, WebviewPanel } from 'vsc import * as uuid from 'uuid/v4'; import { createCodeCell, createErrorOutput } from '../../../datascience-ui/common/cellFactory'; -import { CursorPos } from '../../../datascience-ui/interactive-common/mainState'; -import { CommonActionType } from '../../../datascience-ui/interactive-common/redux/reducers/types'; import { IApplicationShell, ICommandManager, IDocumentManager, ILiveShareApi, IWebPanelProvider, IWorkspaceService } from '../../common/application/types'; import { ContextKey } from '../../common/contextKey'; import { traceError } from '../../common/logger'; @@ -242,10 +240,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { } public addCellBelow() { - const newCellId = uuid(); - this.postMessage(InteractiveWindowMessages.NotebookAddCellBelow, { newCellId }).ignoreErrors(); - // tslint:disable-next-line: no-any - this.postMessage(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Top } as any).ignoreErrors(); + this.postMessage(InteractiveWindowMessages.NotebookAddCellBelow, { newCellId: uuid() }).ignoreErrors(); } protected addSysInfo(_reason: SysInfoReason): Promise { diff --git a/src/datascience-ui/native-editor/redux/reducers/index.ts b/src/datascience-ui/native-editor/redux/reducers/index.ts index b0e353ac8b14..46f9afd3fa66 100644 --- a/src/datascience-ui/native-editor/redux/reducers/index.ts +++ b/src/datascience-ui/native-editor/redux/reducers/index.ts @@ -68,7 +68,7 @@ export const reducerMap: Partial = { [InteractiveWindowMessages.LoadAllCells]: Creation.loadAllCells, [InteractiveWindowMessages.NotebookRunAllCells]: Execution.executeAllCells, [InteractiveWindowMessages.NotebookRunSelectedCell]: Execution.executeSelectedCell, - [InteractiveWindowMessages.NotebookAddCellBelow]: Creation.addNewCell, + [InteractiveWindowMessages.NotebookAddCellBelow]: Creation.addAndFocusCell, [InteractiveWindowMessages.DoSave]: Transfer.save, [InteractiveWindowMessages.DeleteAllCells]: Creation.deleteAllCells, [InteractiveWindowMessages.Undo]: Execution.undo, From ffd5e3a9ff5cdde09f99671a1905335fbc83f30c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Feb 2020 17:49:44 -0800 Subject: [PATCH 09/12] Address code review comments --- .../datascience/interactive-common/interactiveWindowTypes.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/client/datascience/interactive-common/interactiveWindowTypes.ts b/src/client/datascience/interactive-common/interactiveWindowTypes.ts index 4e062856c5ca..d953f367cd60 100644 --- a/src/client/datascience/interactive-common/interactiveWindowTypes.ts +++ b/src/client/datascience/interactive-common/interactiveWindowTypes.ts @@ -4,7 +4,7 @@ import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api'; import { Uri } from 'vscode'; import { IServerState } from '../../../datascience-ui/interactive-common/mainState'; -import { CommonActionType, IAddCellAction, ICellAndCursorAction } from '../../../datascience-ui/interactive-common/redux/reducers/types'; +import { CommonActionType, IAddCellAction } from '../../../datascience-ui/interactive-common/redux/reducers/types'; import { PythonInterpreter } from '../../interpreter/contracts'; import { LiveKernelModel } from '../jupyter/kernels/types'; import { CssMessages, IGetCssRequest, IGetCssResponse, IGetMonacoThemeRequest, SharedMessages } from '../messages'; @@ -502,7 +502,6 @@ export class IInteractiveWindowMapping { public [InteractiveWindowMessages.NotebookRunAllCells]: never | undefined; public [InteractiveWindowMessages.NotebookRunSelectedCell]: never | undefined; public [InteractiveWindowMessages.NotebookAddCellBelow]: IAddCellAction; - public [CommonActionType.FOCUS_CELL]: ICellAndCursorAction; public [InteractiveWindowMessages.DoSave]: never | undefined; public [InteractiveWindowMessages.ExecutionRendered]: IRenderComplete; public [InteractiveWindowMessages.FocusedCellEditor]: IFocusedCellEditor; From efd9fe8960ba43cba877765c1c2d3d3de38a67c2 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Feb 2020 17:52:05 -0800 Subject: [PATCH 10/12] Fix tests --- src/test/datascience/nativeEditor.functional.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index 8a98c76fd332..62ef4174c229 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -806,7 +806,7 @@ for _ in range(50): await update; // Monaco editor should be rendered and the cell should be markdown - assert.ok(isCellFocused(wrapper, 'NativeCell', 0)); + assert.ok(!isCellFocused(wrapper, 'NativeCell', 0)); assert.ok(isCellMarkdown(wrapper, 'NativeCell', 0)); assert.equal( wrapper @@ -1356,7 +1356,7 @@ for _ in range(50): await update; // Monaco editor should be rendered and the cell should be markdown - assert.ok(isCellFocused(wrapper, 'NativeCell', 1), '1st cell is not focused'); + assert.ok(!isCellFocused(wrapper, 'NativeCell', 1), '1st cell is not focused'); assert.ok(isCellMarkdown(wrapper, 'NativeCell', 1), '1st cell is not markdown'); assert.equal( wrapper @@ -1398,7 +1398,7 @@ for _ in range(50): wrapperElement.simulate('keyDown', { key: 'y' }); await update; - assert.ok(isCellFocused(wrapper, 'NativeCell', 1), '1st cell is not focused 2nd time'); + assert.ok(!isCellFocused(wrapper, 'NativeCell', 1), '1st cell is not focused 2nd time'); assert.ok(!isCellMarkdown(wrapper, 'NativeCell', 1), '1st cell is markdown second time'); // Confirm editor still has the same text From c07ecaec2b95edc80053c76c3a8bf031e7a2382e Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 7 Feb 2020 17:55:20 -0800 Subject: [PATCH 11/12] Retry flaky test 3 times. --- src/test/datascience/nativeEditor.functional.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index 62ef4174c229..aba8fbbbbaca 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -1088,7 +1088,7 @@ for _ in range(50): // Confirm it is no longer focused, and it is selected. assert.equal(isCellSelected(wrapper, 'NativeCell', 1), true); assert.equal(isCellFocused(wrapper, 'NativeCell', 1), false); - }); + }).retries(3); test("Pressing 'Shift+Enter' on a selected cell executes the cell and advances to the next cell", async () => { let update = waitForUpdate(wrapper, NativeEditor, 1); From 3b4d1db18d3d5b25b09bf9252597861c4215a7ca Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Sat, 8 Feb 2020 12:57:09 -0800 Subject: [PATCH 12/12] Fixes --- .../interactive-common/redux/store.ts | 3 + .../nativeEditor.functional.test.tsx | 96 +++++++++++++------ src/test/datascience/testHelpers.tsx | 16 +++- 3 files changed, 82 insertions(+), 33 deletions(-) diff --git a/src/datascience-ui/interactive-common/redux/store.ts b/src/datascience-ui/interactive-common/redux/store.ts index f3c0668c8cb2..7fdd655335f6 100644 --- a/src/datascience-ui/interactive-common/redux/store.ts +++ b/src/datascience-ui/interactive-common/redux/store.ts @@ -167,6 +167,9 @@ function createTestMiddleware(): Redux.Middleware<{}, IStore> { sendMessage(InteractiveWindowMessages.ExecutionRendered, { ids: diff }); } + if (action.type !== 'action.postOutgoingMessage') { + sendMessage(`DISPATCHED_ACTION_${action.type}`, {}); + } return res; }; } diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index aba8fbbbbaca..952dbeb96eb5 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -4,6 +4,7 @@ import { nbformat } from '@jupyterlab/coreutils'; import { assert, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; +import * as dedent from 'dedent'; import { ReactWrapper } from 'enzyme'; import { EventEmitter } from 'events'; import * as fs from 'fs-extra'; @@ -22,6 +23,7 @@ import { JupyterExecutionFactory } from '../../client/datascience/jupyter/jupyte import { ICell, IDataScienceErrorHandler, IJupyterExecution, INotebookEditorProvider, INotebookExporter } from '../../client/datascience/types'; import { PythonInterpreter } from '../../client/interpreter/contracts'; import { Editor } from '../../datascience-ui/interactive-common/editor'; +import { CommonActionType } from '../../datascience-ui/interactive-common/redux/reducers/types'; import { NativeCell } from '../../datascience-ui/native-editor/nativeCell'; import { NativeEditor } from '../../datascience-ui/native-editor/nativeEditor'; import { IKeyboardEvent } from '../../datascience-ui/react-common/event'; @@ -128,26 +130,26 @@ suite('DataScience Native Editor', () => { // Create an editor so something is listening to messages await createNewEditor(ioc); - const badPanda = `import pandas as pd -df = pd.read("${escapePath(path.join(srcDirectory(), 'DefaultSalesReport.csv'))}") -df.head()`; - const goodPanda = `import pandas as pd -df = pd.read_csv("${escapePath(path.join(srcDirectory(), 'DefaultSalesReport.csv'))}") -df.head()`; + const badPanda = dedent`import pandas as pd + df = pd.read("${escapePath(path.join(srcDirectory(), 'DefaultSalesReport.csv'))}") + df.head()`; + const goodPanda = dedent`import pandas as pd + df = pd.read_csv("${escapePath(path.join(srcDirectory(), 'DefaultSalesReport.csv'))}") + df.head()`; const matPlotLib = 'import matplotlib.pyplot as plt\r\nimport numpy as np\r\nx = np.linspace(0,20,100)\r\nplt.plot(x, np.sin(x))\r\nplt.show()'; const matPlotLibResults = 'img'; - const spinningCursor = `import sys -import time -def spinning_cursor(): - while True: - for cursor in '|/-\\\\': - yield cursor -spinner = spinning_cursor() -for _ in range(50): - sys.stdout.write(next(spinner)) - sys.stdout.flush() - time.sleep(0.1) - sys.stdout.write('\\r')`; + const spinningCursor = dedent`import sys + import time + def spinning_cursor(): + while True: + for cursor in '|/-\\\\': + yield cursor + spinner = spinning_cursor() + for _ in range(50): + sys.stdout.write(next(spinner)) + sys.stdout.flush() + time.sleep(0.1) + sys.stdout.write('\\r')`; const alternating = `from IPython.display import display\r\nprint('foo')\r\ndisplay('foo')\r\nprint('bar')\r\ndisplay('bar')`; const alternatingResults = ['foo\n', 'foo', 'bar\n', 'bar']; @@ -499,7 +501,7 @@ for _ in range(50): } ); - test('Failure', async () => { + test('xFailure', async () => { let fail = true; const errorThrownDeferred = createDeferred(); // Make a dummy class that will fail during launch @@ -536,7 +538,7 @@ for _ in range(50): assert.equal(imageButtons.length, 6, 'Cell buttons not found'); const runButton = imageButtons.findWhere(w => w.props().tooltip === 'Run cell'); assert.equal(runButton.length, 1, 'No run button found'); - const update = waitForMessage(ioc, InteractiveWindowMessages.ExecutionRendered); + const update = waitForMessage(ioc, InteractiveWindowMessages.ExecutionRendered, { numberOfTimes: 2 }); runButton.simulate('click'); await update; verifyHtmlOnCell(wrapper, 'NativeCell', `1`, 1); @@ -801,13 +803,20 @@ for _ in range(50): clickCell(0); // Switch to markdown - let update = waitForMessage(ioc, InteractiveWindowMessages.FocusedCellEditor); + let update = waitForMessage(ioc, CommonActionType.CHANGE_CELL_TYPE); simulateKeyPressOnCell(0, { code: 'm' }); await update; // Monaco editor should be rendered and the cell should be markdown assert.ok(!isCellFocused(wrapper, 'NativeCell', 0)); assert.ok(isCellMarkdown(wrapper, 'NativeCell', 0)); + + // Focus the cell. + update = waitForUpdate(wrapper, NativeEditor, 1); + simulateKeyPressOnCell(0, { code: 'Enter', editorInfo: undefined }); + await update; + + assert.ok(isCellFocused(wrapper, 'NativeCell', 0)); assert.equal( wrapper .find(NativeCell) @@ -1347,17 +1356,43 @@ for _ in range(50): assert.equal(optionsUpdated.lastCall.args[0].lineNumbers, 'off'); }); - test("Toggle markdown and code modes using 'y' and 'm' keys", async () => { + test("Toggle markdown and code modes using 'y' and 'm' keys (cells should not be focused)", async () => { clickCell(1); + // Switch to markdown + let update = waitForMessage(ioc, CommonActionType.CHANGE_CELL_TYPE); + simulateKeyPressOnCell(1, { code: 'm' }); + await update; + // Monaco editor should be rendered and the cell should be markdown + assert.ok(!isCellFocused(wrapper, 'NativeCell', 1), '1st cell is not focused'); + assert.ok(isCellMarkdown(wrapper, 'NativeCell', 1), '1st cell is not markdown'); + + // Switch to code + update = waitForMessage(ioc, CommonActionType.CHANGE_CELL_TYPE); + simulateKeyPressOnCell(1, { code: 'y' }); + await update; + + assert.ok(!isCellFocused(wrapper, 'NativeCell', 1), '1st cell is not focused 2nd time'); + assert.ok(!isCellMarkdown(wrapper, 'NativeCell', 1), '1st cell is markdown second time'); + }); + + test("Toggle markdown and code modes using 'y' and 'm' keys & ensure changes to cells is preserved", async () => { + clickCell(1); // Switch to markdown - let update = waitForMessage(ioc, InteractiveWindowMessages.FocusedCellEditor); + let update = waitForMessage(ioc, CommonActionType.CHANGE_CELL_TYPE); simulateKeyPressOnCell(1, { code: 'm' }); await update; // Monaco editor should be rendered and the cell should be markdown assert.ok(!isCellFocused(wrapper, 'NativeCell', 1), '1st cell is not focused'); assert.ok(isCellMarkdown(wrapper, 'NativeCell', 1), '1st cell is not markdown'); + + // Focus the cell. + update = waitForUpdate(wrapper, NativeEditor, 1); + simulateKeyPressOnCell(1, { code: 'Enter', editorInfo: undefined }); + await update; + + assert.ok(isCellFocused(wrapper, 'NativeCell', 1)); assert.equal( wrapper .find(NativeCell) @@ -1388,19 +1423,18 @@ for _ in range(50): ); // Switch to code - update = waitForMessage(ioc, InteractiveWindowMessages.FocusedCellEditor); - // At this moment, there's no cell input element, hence send key strokes to the wrapper. - const wrapperElement = wrapper - .find(NativeCell) - .at(1) - .find('.cell-wrapper') - .first(); - wrapperElement.simulate('keyDown', { key: 'y' }); + update = waitForMessage(ioc, CommonActionType.CHANGE_CELL_TYPE); + simulateKeyPressOnCell(1, { code: 'y' }); await update; assert.ok(!isCellFocused(wrapper, 'NativeCell', 1), '1st cell is not focused 2nd time'); assert.ok(!isCellMarkdown(wrapper, 'NativeCell', 1), '1st cell is markdown second time'); + // Focus the cell. + update = waitForUpdate(wrapper, NativeEditor, 1); + simulateKeyPressOnCell(1, { code: 'Enter', editorInfo: undefined }); + await update; + // Confirm editor still has the same text editor = getNativeFocusedEditor(wrapper); const monacoEditor = editor!.instance() as MonacoEditor; diff --git a/src/test/datascience/testHelpers.tsx b/src/test/datascience/testHelpers.tsx index 7b21c6ab57d3..626ea156ca44 100644 --- a/src/test/datascience/testHelpers.tsx +++ b/src/test/datascience/testHelpers.tsx @@ -81,8 +81,9 @@ export function waitForMessage(ioc: DataScienceIocContainer, message: string, op }, timeoutMs) : undefined; let timesMessageReceived = 0; + const dispatchedAction = `DISPATCHED_ACTION_${message}`; handler = (m: string, _p: any) => { - if (m === message) { + if (m === message || m === dispatchedAction) { timesMessageReceived += 1; if (timesMessageReceived < numberOfTimes) { return; @@ -95,7 +96,18 @@ export function waitForMessage(ioc: DataScienceIocContainer, message: string, op if (ioc.wrapper) { ioc.wrapper.update(); } - promise.resolve(); + if (m === message) { + promise.resolve(); + } else { + // It could a redux dispatched message. + // Wait for 10ms, wait for other stuff to finish. + // We can wait for 100ms or 1s. But thats too long. + // The assumption is that currently we do not have any setTimeouts + // in UI code that's in the magnitude of 100ms or more. + // We do have a couple of setTiemout's, but they wait for 1ms, not 100ms. + // 10ms more than sufficient for all the UI timeouts. + setTimeout(() => promise.resolve(), 10); + } } }; ioc.addMessageListener(handler);