Skip to content

Commit 9d72014

Browse files
DonJayamanneDonJayamanne
committed
Fixes
1 parent 05bf2ea commit 9d72014

File tree

10 files changed

+102
-68
lines changed

10 files changed

+102
-68
lines changed

‎src/client/datascience/interactive-common/interactiveWindowTypes.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
'use strict';
44
import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api';
55
import { IServerState } from '../../../datascience-ui/interactive-common/mainState';
6-
import { IAddCellAction, ICellAction } from '../../../datascience-ui/interactive-common/redux/reducers/types';
6+
import { CommonActionType, IAddCellAction, ICellAction, ICellAndCursorAction } from '../../../datascience-ui/interactive-common/redux/reducers/types';
77
import { CssMessages, IGetCssRequest, IGetCssResponse, IGetMonacoThemeRequest, SharedMessages } from '../messages';
88
import { IGetMonacoThemeResponse } from '../monacoMessages';
99
import { ICell, IInteractiveWindowInfo, IJupyterVariable, IJupyterVariablesRequest, IJupyterVariablesResponse } from '../types';
@@ -373,6 +373,7 @@ export class IInteractiveWindowMapping {
373373
public [InteractiveWindowMessages.NotebookRunAllCells]: never | undefined;
374374
public [InteractiveWindowMessages.NotebookRunSelectedCell]: never | undefined;
375375
public [InteractiveWindowMessages.NotebookAddCellBelow]: IAddCellAction;
376+
public [CommonActionType.FOCUS_CELL]: ICellAndCursorAction;
376377
public [InteractiveWindowMessages.DoSave]: never | undefined;
377378
public [InteractiveWindowMessages.ExecutionRendered]: IRenderComplete;
378379
public [InteractiveWindowMessages.FocusedCellEditor]: IFocusedCellEditor;

‎src/client/datascience/interactive-ipynb/nativeEditor.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ import { inject, injectable, multiInject, named } from 'inversify';
77
import * as path from 'path';
88
import { Event, EventEmitter, Memento, Uri, ViewColumn, WebviewPanel } from 'vscode';
99

10+
import * as uuid from 'uuid/v4';
1011
import { createCodeCell, createErrorOutput } from '../../../datascience-ui/common/cellFactory';
12+
import { CursorPos } from '../../../datascience-ui/interactive-common/mainState';
13+
import { CommonActionType } from '../../../datascience-ui/interactive-common/redux/reducers/types';
1114
import { IApplicationShell, ICommandManager, IDocumentManager, ILiveShareApi, IWebPanelProvider, IWorkspaceService } from '../../common/application/types';
1215
import { ContextKey } from '../../common/contextKey';
1316
import { traceError } from '../../common/logger';
@@ -273,7 +276,10 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
273276
}
274277

275278
public addCellBelow() {
276-
this.postMessage(InteractiveWindowMessages.NotebookAddCellBelow).ignoreErrors();
279+
const newCellId = uuid();
280+
this.postMessage(InteractiveWindowMessages.NotebookAddCellBelow, { newCellId }).ignoreErrors();
281+
// tslint:disable-next-line: no-any
282+
this.postMessage(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Top } as any).ignoreErrors();
277283
}
278284

279285
public async removeAllCells(): Promise<void> {

‎src/datascience-ui/interactive-common/redux/reducers/types.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
'use strict';
44
import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api';
55

6-
import { InteractiveWindowMessages, IShowDataViewer, NativeCommandType } from '../../../../client/datascience/interactive-common/interactiveWindowTypes';
6+
import { IInteractiveWindowMapping, InteractiveWindowMessages, IShowDataViewer, NativeCommandType } from '../../../../client/datascience/interactive-common/interactiveWindowTypes';
77
import { BaseReduxActionPayload } from '../../../../client/datascience/interactive-common/types';
88
import { IJupyterVariablesRequest } from '../../../../client/datascience/types';
99
import { ActionWithPayload, ReducerArg } from '../../../react-common/reduxUtils';
@@ -73,7 +73,7 @@ export type CommonActionTypeMapping = {
7373
[CommonActionType.INSERT_BELOW]: ICellAction & IAddCellAction;
7474
[CommonActionType.INSERT_ABOVE_FIRST]: IAddCellAction;
7575
[CommonActionType.FOCUS_CELL]: ICellAndCursorAction;
76-
[CommonActionType.UNFOCUS_CELL]: ICodeAction;
76+
[CommonActionType.UNFOCUS_CELL]: ICellAction | ICodeAction;
7777
[CommonActionType.ADD_NEW_CELL]: IAddCellAction;
7878
[CommonActionType.EDIT_CELL]: IEditCellAction;
7979
[CommonActionType.EXECUTE_CELL]: IExecuteAction;
@@ -147,14 +147,9 @@ export interface IEditCellAction extends ICodeAction {
147147

148148
// I.e. when using the operation `add`, we need the corresponding `IAddCellAction`.
149149
// They are mutually exclusive, if not `add`, then there's no `newCellId`.
150-
export type IExecuteAction =
151-
| (ICodeAction & {
152-
moveOp: 'select' | 'none';
153-
})
154-
| (ICodeAction &
155-
IAddCellAction & {
156-
moveOp: 'add';
157-
});
150+
export type IExecuteAction = ICodeAction & {
151+
moveOp: 'select' | 'none' | 'add';
152+
};
158153

159154
export interface ICodeCreatedAction extends ICellAction {
160155
modelId: string;
@@ -181,9 +176,18 @@ export interface IChangeCellTypeAction {
181176
}
182177
export type CommonAction<T = never | undefined> = ActionWithPayload<T, CommonActionType | InteractiveWindowMessages>;
183178

184-
export function createIncomingActionWithPayload<T>(type: CommonActionType | InteractiveWindowMessages, data: T): CommonAction<T> {
179+
/*
180+
Create a type which has `unknown` for all `CustomActionType` items that do not have a corresponding entry in `CommonActionTypeMapping`.
181+
This provides the ability for strong typeing of actions created for a specific command type.
182+
*/
183+
type UnknownTypeForMissingMappings = CommonActionTypeMapping & { [W in CommonActionType]: unknown };
184+
185+
export function createIncomingActionWithPayload<K extends CommonActionType, V extends UnknownTypeForMissingMappings[K]>(type: K, data: V): CommonAction<V>;
186+
export function createIncomingActionWithPayload<T extends IInteractiveWindowMapping, K extends InteractiveWindowMessages, P extends T[K]>(type: K, data: P): CommonAction<P>;
187+
// tslint:disable-next-line: no-any
188+
export function createIncomingActionWithPayload(type: any, data: any): CommonAction<any> {
185189
// tslint:disable-next-line: no-any
186-
return { type, payload: ({ data, messageDirection: 'incoming' } as any) as BaseReduxActionPayload<T> };
190+
return { type, payload: ({ data, messageDirection: 'incoming' } as any) as BaseReduxActionPayload<any> } as any;
187191
}
188192
export function createIncomingAction(type: CommonActionType | InteractiveWindowMessages): CommonAction {
189193
return { type, payload: { messageDirection: 'incoming', data: undefined } };

‎src/datascience-ui/native-editor/nativeCell.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { Image, ImageName } from '../react-common/image';
2525
import { ImageButton } from '../react-common/imageButton';
2626
import { getLocString } from '../react-common/locReactSide';
2727
import { AddCellLine } from './addCellLine';
28-
import { actionCreators } from './redux/actions';
28+
import { ActionCreators, mapDispatchToProps } from './redux/actions';
2929

3030
interface INativeCellBaseProps {
3131
role?: string;
@@ -44,7 +44,7 @@ interface INativeCellBaseProps {
4444
themeMatplotlibPlots: boolean | undefined;
4545
}
4646

47-
type INativeCellProps = INativeCellBaseProps & typeof actionCreators;
47+
type INativeCellProps = INativeCellBaseProps & ActionCreators;
4848

4949
// tslint:disable: react-this-binding-issue
5050
export class NativeCell extends React.Component<INativeCellProps> {
@@ -689,5 +689,5 @@ export class NativeCell extends React.Component<INativeCellProps> {
689689

690690
// Main export, return a redux connected editor
691691
export function getConnectedNativeCell() {
692-
return connect(null, actionCreators)(NativeCell);
692+
return connect(null, mapDispatchToProps)(NativeCell);
693693
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ import { Progress } from '../react-common/progress';
2121
import { AddCellLine } from './addCellLine';
2222
import { getConnectedNativeCell } from './nativeCell';
2323
import './nativeEditor.less';
24-
import { actionCreators } from './redux/actions';
24+
import { ActionCreators, mapDispatchToProps } from './redux/actions';
2525

26-
type INativeEditorProps = IMainWithVariables & typeof actionCreators;
26+
type INativeEditorProps = IMainWithVariables & ActionCreators;
2727

2828
function mapStateToProps(state: IStore): IMainWithVariables {
2929
return { ...state.main, variableState: state.variables };
@@ -414,5 +414,5 @@ export class NativeEditor extends React.Component<INativeEditorProps> {
414414

415415
// Main export, return a redux connected editor
416416
export function getConnectedNativeEditor() {
417-
return connect(mapStateToProps, actionCreators)(NativeEditor);
417+
return connect(mapStateToProps, mapDispatchToProps)(NativeEditor);
418418
}

‎src/datascience-ui/native-editor/redux/actions.ts

Lines changed: 60 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33
'use strict';
44
import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api';
5+
import { bindActionCreators, Dispatch } from 'redux';
56
import * as uuid from 'uuid/v4';
67
import { InteractiveWindowMessages, NativeCommandType } from '../../../client/datascience/interactive-common/interactiveWindowTypes';
78
import { IJupyterVariable, IJupyterVariablesRequest } from '../../../client/datascience/types';
@@ -11,40 +12,72 @@ import {
1112
CommonActionType,
1213
createIncomingAction,
1314
createIncomingActionWithPayload,
14-
IAddCellAction,
1515
ICellAction,
1616
ICellAndCursorAction,
17-
IChangeCellTypeAction,
1817
ICodeAction,
1918
ICodeCreatedAction,
2019
IEditCellAction,
21-
IExecuteAction,
2220
ILinkClickAction,
2321
IRefreshVariablesAction,
2422
ISendCommandAction,
2523
IShowDataViewerAction
2624
} from '../../interactive-common/redux/reducers/types';
2725

26+
const actionCreatorsWithDispatch = (dispatch: Dispatch) => {
27+
return {
28+
addCell: () => {
29+
const newCellId = uuid();
30+
dispatch(createIncomingActionWithPayload(CommonActionType.ADD_NEW_CELL, { newCellId }));
31+
dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current }));
32+
},
33+
insertAboveFirst: () => {
34+
const newCellId = uuid();
35+
dispatch(createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE_FIRST, { newCellId }));
36+
dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current }));
37+
},
38+
insertAbove: (cellId: string | undefined) => {
39+
// Use settimeout to ensure the newly created cell does not end up with the key that was entered by user.
40+
// 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`.
41+
setTimeout(() => {
42+
const newCellId = uuid();
43+
dispatch(createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE, { cellId, newCellId }));
44+
dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current }));
45+
}, 1);
46+
},
47+
insertBelow: (cellId: string | undefined) => {
48+
if (!cellId) {
49+
return;
50+
}
51+
// Use settimeout to ensure the newly created cell does not end up with the key that was entered by user.
52+
// 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`.
53+
setTimeout(() => {
54+
const newCellId = uuid();
55+
dispatch(createIncomingActionWithPayload(CommonActionType.INSERT_BELOW, { cellId, newCellId }));
56+
dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current }));
57+
}, 1);
58+
},
59+
executeCell: (cellId: string, code: string, moveOp: 'add' | 'select' | 'none') => {
60+
dispatch(createIncomingActionWithPayload(CommonActionType.EXECUTE_CELL, { cellId, code, moveOp }));
61+
if (moveOp === 'add') {
62+
const newCellId = uuid();
63+
dispatch(createIncomingActionWithPayload(CommonActionType.INSERT_BELOW, { cellId, newCellId }));
64+
dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId: newCellId, cursorPos: CursorPos.Current }));
65+
}
66+
},
67+
changeCellType: (cellId: string, currentCode: string) => {
68+
dispatch(createIncomingActionWithPayload(CommonActionType.CHANGE_CELL_TYPE, { cellId, currentCode }));
69+
dispatch(createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId, cursorPos: CursorPos.Current }));
70+
}
71+
};
72+
};
73+
2874
// See https://react-redux.js.org/using-react-redux/connect-mapdispatch#defining-mapdispatchtoprops-as-an-object
29-
export const actionCreators = {
30-
insertAbove: (cellId: string | undefined): CommonAction<ICellAction & IAddCellAction> =>
31-
createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE, { cellId, newCellId: uuid() }),
32-
insertAboveFirst: (): CommonAction<IAddCellAction> => createIncomingActionWithPayload(CommonActionType.INSERT_ABOVE_FIRST, { newCellId: uuid() }),
33-
insertBelow: (cellId: string | undefined): CommonAction<ICellAction & IAddCellAction> =>
34-
createIncomingActionWithPayload(CommonActionType.INSERT_BELOW, { cellId, newCellId: uuid() }),
75+
const actionCreators = {
3576
focusCell: (cellId: string, cursorPos: CursorPos = CursorPos.Current): CommonAction<ICellAndCursorAction> =>
3677
createIncomingActionWithPayload(CommonActionType.FOCUS_CELL, { cellId, cursorPos }),
3778
unfocusCell: (cellId: string, code: string): CommonAction<ICodeAction> => createIncomingActionWithPayload(CommonActionType.UNFOCUS_CELL, { cellId, code }),
3879
selectCell: (cellId: string, cursorPos: CursorPos = CursorPos.Current): CommonAction<ICellAndCursorAction> =>
3980
createIncomingActionWithPayload(CommonActionType.SELECT_CELL, { cellId, cursorPos }),
40-
addCell: (): CommonAction<IAddCellAction> => createIncomingActionWithPayload(CommonActionType.ADD_NEW_CELL, { newCellId: uuid() }),
41-
executeCell: (cellId: string, code: string, moveOp: 'add' | 'select' | 'none'): CommonAction<IExecuteAction> => {
42-
if (moveOp === 'add') {
43-
return createIncomingActionWithPayload(CommonActionType.EXECUTE_CELL, { cellId, code, moveOp, newCellId: uuid() });
44-
} else {
45-
return createIncomingActionWithPayload(CommonActionType.EXECUTE_CELL, { cellId, code, moveOp });
46-
}
47-
},
4881
executeAllCells: (): CommonAction => createIncomingAction(CommonActionType.EXECUTE_ALL_CELLS),
4982
executeAbove: (cellId: string): CommonAction<ICellAction> => createIncomingActionWithPayload(CommonActionType.EXECUTE_ABOVE, { cellId }),
5083
executeCellAndBelow: (cellId: string, code: string): CommonAction<ICodeAction> => createIncomingActionWithPayload(CommonActionType.EXECUTE_CELL_AND_BELOW, { cellId, code }),
@@ -62,8 +95,7 @@ export const actionCreators = {
6295
createIncomingActionWithPayload(CommonActionType.SEND_COMMAND, { command, commandType }),
6396
moveCellUp: (cellId: string): CommonAction<ICellAction> => createIncomingActionWithPayload(CommonActionType.MOVE_CELL_UP, { cellId }),
6497
moveCellDown: (cellId: string): CommonAction<ICellAction> => createIncomingActionWithPayload(CommonActionType.MOVE_CELL_DOWN, { cellId }),
65-
changeCellType: (cellId: string, currentCode: string): CommonAction<IChangeCellTypeAction> =>
66-
createIncomingActionWithPayload(CommonActionType.CHANGE_CELL_TYPE, { cellId, currentCode }),
98+
// changeCellType: (cellId: string, currentCode: string) => createIncomingActionWithPayload(CommonActionType.CHANGE_CELL_TYPE, { cellId, currentCode }),
6799
toggleLineNumbers: (cellId: string): CommonAction<ICellAction> => createIncomingActionWithPayload(CommonActionType.TOGGLE_LINE_NUMBERS, { cellId }),
68100
toggleOutput: (cellId: string): CommonAction<ICellAction> => createIncomingActionWithPayload(CommonActionType.TOGGLE_OUTPUT, { cellId }),
69101
deleteCell: (cellId: string): CommonAction<ICellAction> => createIncomingActionWithPayload(InteractiveWindowMessages.DeleteCell, { cellId }),
@@ -86,3 +118,12 @@ export const actionCreators = {
86118
getVariableData: (newExecutionCount: number, startIndex: number = 0, pageSize: number = 100): CommonAction<IJupyterVariablesRequest> =>
87119
createIncomingActionWithPayload(CommonActionType.GET_VARIABLE_DATA, { executionCount: newExecutionCount, sortColumn: 'name', sortAscending: true, startIndex, pageSize })
88120
};
121+
122+
export type ActionCreators = typeof actionCreators & ReturnType<typeof actionCreatorsWithDispatch>;
123+
124+
export function mapDispatchToProps(dispatch: Dispatch): ActionCreators {
125+
return {
126+
...actionCreatorsWithDispatch(dispatch),
127+
...bindActionCreators(actionCreators, dispatch)
128+
};
129+
}

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

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { createCellVM, createEmptyCell, CursorPos, extractInputText, getSelected
88
import { createPostableAction } from '../../../interactive-common/redux/postOffice';
99
import { Helpers } from '../../../interactive-common/redux/reducers/helpers';
1010
import { IAddCellAction, ICellAction } from '../../../interactive-common/redux/reducers/types';
11-
import { actionCreators } from '../actions';
1211
import { NativeEditorReducerArg } from '../mapping';
1312

1413
export namespace Creation {
@@ -60,11 +59,6 @@ export namespace Creation {
6059
createPostableAction(InteractiveWindowMessages.InsertCell, { cell: newVM.cell, index: position, code: '', codeCellAboveId: findFirstCodeCellAbove(newList, position) })
6160
);
6261

63-
// Queue up an action to set focus to the cell we're inserting
64-
setTimeout(() => {
65-
arg.queueAction(actionCreators.focusCell(newVM.cell.id));
66-
});
67-
6862
return result;
6963
}
7064

@@ -95,11 +89,6 @@ export namespace Creation {
9589
createPostableAction(InteractiveWindowMessages.InsertCell, { cell: newVM.cell, index, code: '', codeCellAboveId: findFirstCodeCellAbove(newList, position) })
9690
);
9791

98-
// Queue up an action to set focus to the cell we're inserting
99-
setTimeout(() => {
100-
arg.queueAction(actionCreators.focusCell(newVM.cell.id));
101-
});
102-
10392
return result;
10493
}
10594

‎src/datascience-ui/native-editor/redux/reducers/effects.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export namespace Effects {
5151
return arg.prevState;
5252
}
5353

54-
export function unfocusCell(arg: NativeEditorReducerArg<ICodeAction>): IMainState {
54+
export function unfocusCell(arg: NativeEditorReducerArg<ICellAction | ICodeAction>): IMainState {
5555
// Unfocus the cell
5656
const index = arg.prevState.cellVMs.findIndex(c => c.cell.id === arg.payload.data.cellId);
5757
const selectionInfo = getSelectedAndFocusedInfo(arg.prevState);
@@ -60,13 +60,13 @@ export namespace Effects {
6060
const current = arg.prevState.cellVMs[index];
6161
const newCell = {
6262
...current,
63-
inputBlockText: arg.payload.data.code,
63+
inputBlockText: 'code' in arg.payload.data ? arg.payload.data.code : current.inputBlockText,
6464
focused: false,
6565
cell: {
6666
...current.cell,
6767
data: {
6868
...current.cell.data,
69-
source: arg.payload.data.code
69+
source: 'code' in arg.payload.data ? arg.payload.data.code : current.cell.data.source
7070
}
7171
}
7272
};
@@ -84,12 +84,12 @@ export namespace Effects {
8484
const current = arg.prevState.cellVMs[index];
8585
const newCell = {
8686
...current,
87-
inputBlockText: arg.payload.data.code,
87+
inputBlockText: 'code' in arg.payload.data ? arg.payload.data.code : current.inputBlockText,
8888
cell: {
8989
...current.cell,
9090
data: {
9191
...current.cell.data,
92-
source: arg.payload.data.code
92+
source: 'code' in arg.payload.data ? arg.payload.data.code : current.cell.data.source
9393
}
9494
}
9595
};

0 commit comments

Comments
 (0)