From f7e660ec8025b08f8c0352294928b75af3cd210b Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 15 Apr 2020 08:45:05 -0700 Subject: [PATCH 1/2] Fix undo in notebooks (#11167) * Put back ctrl+z and 'z' for undo. Refactor to use one value to compute custom editor support * Add news entry --- news/2 Fixes/11160.md | 1 + src/client/datascience/data-viewing/dataViewer.ts | 8 +++++--- .../datascience/interactive-common/interactiveBase.ts | 6 ++++-- src/client/datascience/interactive-ipynb/nativeEditor.ts | 8 +++++--- .../interactive-ipynb/nativeEditorOldWebView.ts | 7 +++++-- .../datascience/interactive-window/interactiveWindow.ts | 7 +++++-- src/client/datascience/plotting/plotViewer.ts | 8 +++++--- src/client/datascience/types.ts | 1 + src/client/datascience/webViewHost.ts | 6 ++++-- src/datascience-ui/native-editor/nativeCell.tsx | 7 ++++--- src/datascience-ui/native-editor/nativeEditor.tsx | 8 ++++++-- src/datascience-ui/react-common/constants.ts | 2 -- src/datascience-ui/react-common/settingsReactSide.ts | 3 ++- src/test/datascience/nativeEditor.functional.test.tsx | 3 --- .../datascience/uiTests/ipywidget.ui.functional.test.ts | 3 --- 15 files changed, 47 insertions(+), 31 deletions(-) create mode 100644 news/2 Fixes/11160.md diff --git a/news/2 Fixes/11160.md b/news/2 Fixes/11160.md new file mode 100644 index 000000000000..e7cc66d6be42 --- /dev/null +++ b/news/2 Fixes/11160.md @@ -0,0 +1 @@ +Fix CTRL+Z and Z for undo on notebooks. \ No newline at end of file diff --git a/src/client/datascience/data-viewing/dataViewer.ts b/src/client/datascience/data-viewing/dataViewer.ts index 77350b95d575..e71a68255533 100644 --- a/src/client/datascience/data-viewing/dataViewer.ts +++ b/src/client/datascience/data-viewing/dataViewer.ts @@ -8,7 +8,7 @@ import * as path from 'path'; import { ViewColumn } from 'vscode'; import { IApplicationShell, IWebPanelProvider, IWorkspaceService } from '../../common/application/types'; -import { EXTENSION_ROOT_DIR } from '../../common/constants'; +import { EXTENSION_ROOT_DIR, UseCustomEditorApi } from '../../common/constants'; import { WebHostNotebook } from '../../common/experimentGroups'; import { traceError } from '../../common/logger'; import { IConfigurationService, IDisposable, IExperimentsManager, Resource } from '../../common/types'; @@ -39,7 +39,8 @@ export class DataViewer extends WebViewHost implements IData @inject(IWorkspaceService) workspaceService: IWorkspaceService, @inject(IJupyterVariables) private variableManager: IJupyterVariables, @inject(IApplicationShell) private applicationShell: IApplicationShell, - @inject(IExperimentsManager) experimentsManager: IExperimentsManager + @inject(IExperimentsManager) experimentsManager: IExperimentsManager, + @inject(UseCustomEditorApi) useCustomEditorApi: boolean ) { super( configuration, @@ -52,7 +53,8 @@ export class DataViewer extends WebViewHost implements IData [path.join(dataExplorereDir, 'commons.initial.bundle.js'), path.join(dataExplorereDir, 'dataExplorer.js')], localize.DataScience.dataExplorerTitle(), ViewColumn.One, - experimentsManager.inExperiment(WebHostNotebook.experiment) + experimentsManager.inExperiment(WebHostNotebook.experiment), + useCustomEditorApi ); // Load the web panel using our current directory as we don't expect to load any other files diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts index 7112f7806f1d..20798c17f491 100644 --- a/src/client/datascience/interactive-common/interactiveBase.ts +++ b/src/client/datascience/interactive-common/interactiveBase.ts @@ -150,7 +150,8 @@ export abstract class InteractiveBase extends WebViewHost implements IPlot @inject(IWorkspaceService) workspaceService: IWorkspaceService, @inject(IApplicationShell) private applicationShell: IApplicationShell, @inject(IFileSystem) private fileSystem: IFileSystem, - @inject(IExperimentsManager) experimentsManager: IExperimentsManager + @inject(IExperimentsManager) experimentsManager: IExperimentsManager, + @inject(UseCustomEditorApi) useCustomEditorApi: boolean ) { super( configuration, @@ -48,7 +49,8 @@ export class PlotViewer extends WebViewHost implements IPlot [path.join(plotDir, 'commons.initial.bundle.js'), path.join(plotDir, 'plotViewer.js')], localize.DataScience.plotViewerTitle(), ViewColumn.One, - experimentsManager.inExperiment(WebHostNotebook.experiment) + experimentsManager.inExperiment(WebHostNotebook.experiment), + useCustomEditorApi ); // Load the web panel using our current directory as we don't expect to load any other files super.loadWebPanel(process.cwd()).catch(traceError); diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index d8b2bf1c4150..4fc71ebb8d10 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -652,6 +652,7 @@ export interface IDataScienceExtraSettings extends IDataScienceSettings { fontFamily: string; }; theme: string; + useCustomEditorApi: boolean; }; intellisenseOptions: { quickSuggestions: { diff --git a/src/client/datascience/webViewHost.ts b/src/client/datascience/webViewHost.ts index a4bd0bc4622f..4138f779e1f9 100644 --- a/src/client/datascience/webViewHost.ts +++ b/src/client/datascience/webViewHost.ts @@ -50,7 +50,8 @@ export abstract class WebViewHost implements IDisposable { @unmanaged() private scripts: string[], @unmanaged() private title: string, @unmanaged() private viewColumn: ViewColumn, - @unmanaged() private readonly useWebViewServer: boolean + @unmanaged() private readonly useWebViewServer: boolean, + @unmanaged() protected readonly useCustomEditorApi: boolean ) { // Create our message listener for our web panel. this.messageListener = messageListenerCtor( @@ -193,7 +194,8 @@ export abstract class WebViewHost implements IDisposable { fontSize: this.getValue(editor, 'fontSize', 14), fontFamily: this.getValue(editor, 'fontFamily', "Consolas, 'Courier New', monospace") }, - theme: theme + theme: theme, + useCustomEditorApi: this.useCustomEditorApi }, intellisenseOptions: { quickSuggestions: { diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index 409054dd7ec9..07ff608fd009 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -19,7 +19,7 @@ import { CellOutput } from '../interactive-common/cellOutput'; import { ExecutionCount } from '../interactive-common/executionCount'; import { InformationMessages } from '../interactive-common/informationMessages'; import { CursorPos, ICellViewModel, IFont } from '../interactive-common/mainState'; -import { getOSType, UseCustomEditor } from '../react-common/constants'; +import { getOSType } from '../react-common/constants'; import { IKeyboardEvent } from '../react-common/event'; import { Image, ImageName } from '../react-common/image'; import { ImageButton } from '../react-common/imageButton'; @@ -51,6 +51,7 @@ interface INativeCellBaseProps { themeMatplotlibPlots: boolean | undefined; focusPending: number; busy: boolean; + useCustomEditorApi: boolean; } type INativeCellProps = INativeCellBaseProps & typeof actionCreators; @@ -356,7 +357,7 @@ export class NativeCell extends React.Component { break; case 'z': case 'Z': - if (!this.isFocused() && !UseCustomEditor.enabled) { + if (!this.isFocused() && !this.props.useCustomEditorApi) { if (e.shiftKey && !e.ctrlKey && !e.altKey) { e.stopPropagation(); this.props.redo(); @@ -657,7 +658,7 @@ export class NativeCell extends React.Component { keyDown={this.keyDownInput} showLineNumbers={this.props.cellVM.showLineNumbers} font={this.props.font} - disableUndoStack={UseCustomEditor.enabled} + disableUndoStack={this.props.useCustomEditorApi} codeVersion={this.props.cellVM.codeVersion ? this.props.cellVM.codeVersion : 1} focusPending={this.props.focusPending} /> diff --git a/src/datascience-ui/native-editor/nativeEditor.tsx b/src/datascience-ui/native-editor/nativeEditor.tsx index 20f8bb0be0e6..0f4b1979f8db 100644 --- a/src/datascience-ui/native-editor/nativeEditor.tsx +++ b/src/datascience-ui/native-editor/nativeEditor.tsx @@ -11,7 +11,7 @@ import { handleLinkClick } from '../interactive-common/handlers'; import { getSelectedAndFocusedInfo, ICellViewModel, IMainState } from '../interactive-common/mainState'; import { IMainWithVariables, IStore } from '../interactive-common/redux/store'; import { IVariablePanelProps, VariablePanel } from '../interactive-common/variablePanel'; -import { getOSType, UseCustomEditor } from '../react-common/constants'; +import { getOSType } from '../react-common/constants'; import { ErrorBoundary } from '../react-common/errorBoundary'; import { getLocString } from '../react-common/locReactSide'; import { Progress } from '../react-common/progress'; @@ -193,7 +193,10 @@ ${buildSettingsCss(this.props.settings)}`} } case 'z': case 'Z': - if (!getSelectedAndFocusedInfo(this.props).focusedCellId && !UseCustomEditor.enabled) { + if ( + !getSelectedAndFocusedInfo(this.props).focusedCellId && + !this.props.settings?.extraSettings.useCustomEditorApi + ) { if (event.shiftKey && !event.ctrlKey && !event.altKey) { event.stopPropagation(); this.props.redo(); @@ -283,6 +286,7 @@ ${buildSettingsCss(this.props.settings)}`} // Focus pending does not apply to native editor. focusPending={0} busy={this.props.busy} + useCustomEditorApi={this.props.settings?.extraSettings.useCustomEditorApi} /> {lastLine} diff --git a/src/datascience-ui/react-common/constants.ts b/src/datascience-ui/react-common/constants.ts index 9ee4d2026375..c7bd757316d7 100644 --- a/src/datascience-ui/react-common/constants.ts +++ b/src/datascience-ui/react-common/constants.ts @@ -23,5 +23,3 @@ export function getOSType() { return OSType.Unknown; } } - -export const UseCustomEditor = { enabled: true }; diff --git a/src/datascience-ui/react-common/settingsReactSide.ts b/src/datascience-ui/react-common/settingsReactSide.ts index 0a3436e30ada..6f40ee0d6782 100644 --- a/src/datascience-ui/react-common/settingsReactSide.ts +++ b/src/datascience-ui/react-common/settingsReactSide.ts @@ -46,7 +46,8 @@ export function getDefaultSettings() { fontSize: 14, fontFamily: "Consolas, 'Courier New', monospace" }, - theme: 'Default Dark+' + theme: 'Default Dark+', + useCustomEditorApi: false }, intellisenseOptions: { quickSuggestions: { diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index 4bdd5d71ce8a..f7e03064e321 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -38,7 +38,6 @@ import { ExecutionCount } from '../../datascience-ui/interactive-common/executio 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 { UseCustomEditor } from '../../datascience-ui/react-common/constants'; import { IKeyboardEvent } from '../../datascience-ui/react-common/event'; import { ImageButton } from '../../datascience-ui/react-common/imageButton'; import { IMonacoEditorState, MonacoEditor } from '../../datascience-ui/react-common/monacoEditor'; @@ -130,7 +129,6 @@ suite('DataScience Native Editor', () => { }; setup(async () => { - UseCustomEditor.enabled = useCustomEditorApi; ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(useCustomEditorApi); await ioc.activate(); @@ -906,7 +904,6 @@ df.head()`; cleanupCallback: Function; }; function initIoc() { - UseCustomEditor.enabled = useCustomEditorApi; ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(useCustomEditorApi); return ioc.activate(); diff --git a/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts b/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts index 8c7a0e2bf632..3e006dfc1c53 100644 --- a/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts +++ b/src/test/datascience/uiTests/ipywidget.ui.functional.test.ts @@ -14,7 +14,6 @@ import * as path from 'path'; import * as sinon from 'sinon'; import { Disposable } from 'vscode'; import { EXTENSION_ROOT_DIR } from '../../../client/constants'; -import { UseCustomEditor } from '../../../datascience-ui/react-common/constants'; import { retryIfFail as retryIfFailOriginal } from '../../common'; import { mockedVSCodeNamespaces } from '../../vscode-mock'; import { DataScienceIocContainer } from '../dataScienceIocContainer'; @@ -37,7 +36,6 @@ use(chaiAsPromised); suiteSetup(function () { // These are UI tests, hence nothing to do with platforms. - UseCustomEditor.enabled = useCustomEditorApi; this.timeout(30_000); // UI Tests, need time to start jupyter. if (!process.env.VSCODE_PYTHON_ROLLING) { // Skip all tests unless using real jupyter @@ -45,7 +43,6 @@ use(chaiAsPromised); } }); setup(async () => { - UseCustomEditor.enabled = useCustomEditorApi; ioc = new DataScienceIocContainer(true); ioc.registerDataScienceTypes(useCustomEditorApi); await ioc.activate(); From b43cf6b3fe38b1063fa430997d084f143173888d Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 15 Apr 2020 08:47:55 -0700 Subject: [PATCH 2/2] Update change log --- CHANGELOG.md | 2 ++ news/2 Fixes/11160.md | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) delete mode 100644 news/2 Fixes/11160.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 79354153f311..a49058b3ce26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -98,6 +98,8 @@ ([#10971](https://github.com/Microsoft/vscode-python/issues/10971)) 1. Fix problem with opening a notebook in jupyter after saving in VS code. ([#11151](https://github.com/Microsoft/vscode-python/issues/11151)) +1. Fix CTRL+Z and Z for undo on notebooks. + ([#11160](https://github.com/Microsoft/vscode-python/issues/11160)) ### Code Health diff --git a/news/2 Fixes/11160.md b/news/2 Fixes/11160.md deleted file mode 100644 index e7cc66d6be42..000000000000 --- a/news/2 Fixes/11160.md +++ /dev/null @@ -1 +0,0 @@ -Fix CTRL+Z and Z for undo on notebooks. \ No newline at end of file