Skip to content

Commit

Permalink
DS: Fixes for trusted notebooks UI (microsoft#12685)
Browse files Browse the repository at this point in the history
* Prompt to trust notebook should be error message

* Fix apparent bug with markdown editor styling

This was causing the markdown cells to jut out 2 pixels to the right of the middle content bar

* Refactor readOnly markdown props for consistency

* Sign up for keydown / up events when props.readOnly changes
  • Loading branch information
joyceerhl committed Jul 1, 2020
1 parent 0ac2757 commit 21fcb0b
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 17 deletions.
2 changes: 1 addition & 1 deletion src/client/datascience/interactive-ipynb/nativeEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {

private async launchNotebookTrustPrompt() {
const prompts = [localize.DataScience.trustNotebook(), localize.DataScience.doNotTrustNotebook()];
const selection = await this.applicationShell.showInformationMessage(
const selection = await this.applicationShell.showErrorMessage(
localize.DataScience.launchNotebookTrustPrompt(),
...prompts
);
Expand Down
2 changes: 1 addition & 1 deletion src/datascience-ui/interactive-common/cellInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ export class CellInput extends React.Component<ICellInputProps> {
<div className="cell-input">
<Markdown
editorOptions={this.props.editorOptions}
readOnly={!this.props.isNotebookTrusted}
markdown={source}
codeTheme={this.props.codeTheme}
testMode={this.props.testMode ? true : false}
Expand All @@ -158,7 +159,6 @@ export class CellInput extends React.Component<ICellInputProps> {
font={this.props.font}
disableUndoStack={this.props.disableUndoStack}
version={this.props.codeVersion}
isNotebookTrusted={this.props.isNotebookTrusted}
/>
</div>
);
Expand Down
4 changes: 4 additions & 0 deletions src/datascience-ui/interactive-common/editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ export class Editor extends React.Component<IEditorProps> {
}
}
}
if (prevProps.readOnly === true && this.props.readOnly === false && this.editorRef) {
this.subscriptions.push(this.editorRef.onKeyDown(this.onKeyDown));
this.subscriptions.push(this.editorRef.onKeyUp(this.onKeyUp));
}
}

public render() {
Expand Down
4 changes: 2 additions & 2 deletions src/datascience-ui/interactive-common/markdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export interface IMarkdownProps {
hasFocus: boolean;
cursorPos: CursorPos | monacoEditor.IPosition;
disableUndoStack: boolean;
isNotebookTrusted: boolean;
readOnly: boolean;
onCreated(code: string, modelId: string): void;
onChange(e: IMonacoModelContentChangeEvent): void;
focused?(): void;
Expand All @@ -47,7 +47,7 @@ export class Markdown extends React.Component<IMarkdownProps> {
<div className={classes}>
<Editor
codeTheme={this.props.codeTheme}
readOnly={!this.props.isNotebookTrusted}
readOnly={this.props.readOnly}
history={undefined}
onCreated={this.props.onCreated}
onChange={this.props.onChange}
Expand Down
20 changes: 8 additions & 12 deletions src/datascience-ui/native-editor/nativeCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,8 @@ export class NativeCell extends React.Component<INativeCellProps> {
};

private renderNormalCell() {
const cellOuterClass =
this.props.cellVM.editable && this.props.isNotebookTrusted ? 'cell-outer-editable' : 'cell-outer';
let cellWrapperClass =
this.props.cellVM.editable && this.props.isNotebookTrusted
? 'cell-wrapper'
: 'cell-wrapper cell-wrapper-noneditable';
const cellOuterClass = this.props.cellVM.editable ? 'cell-outer-editable' : 'cell-outer';
let cellWrapperClass = this.props.cellVM.editable ? 'cell-wrapper' : 'cell-wrapper cell-wrapper-noneditable';
if (this.isSelected() && !this.isFocused()) {
cellWrapperClass += ' cell-wrapper-selected';
}
Expand Down Expand Up @@ -245,7 +241,7 @@ export class NativeCell extends React.Component<INativeCellProps> {
};

private isShowingMarkdownEditor = (): boolean => {
return this.isMarkdownCell() && this.props.cellVM.focused;
return this.isMarkdownCell() && (this.props.cellVM.focused || !this.isNotebookTrusted());
};

private shouldRenderInput(): boolean {
Expand All @@ -265,7 +261,10 @@ export class NativeCell extends React.Component<INativeCellProps> {
};

private shouldRenderOutput(): boolean {
if (this.isCodeCell() && this.props.isNotebookTrusted) {
if (!this.isNotebookTrusted()) {
return false;
}
if (this.isCodeCell()) {
const cell = this.getCodeCell();
return (
this.hasOutput() &&
Expand All @@ -275,16 +274,13 @@ export class NativeCell extends React.Component<INativeCellProps> {
cell.outputs.length !== 0
);
} else if (this.isMarkdownCell()) {
return !this.isShowingMarkdownEditor() && this.isNotebookTrusted();
return !this.isShowingMarkdownEditor();
}
return false;
}

// tslint:disable-next-line: cyclomatic-complexity max-func-body-length
private keyDownInput = (cellId: string, e: IKeyboardEvent) => {
if (!this.isNotebookTrusted()) {
return; // Disable keyboard interaction with untrusted notebooks
}
const isFocusedWhenNotSuggesting = this.isFocused() && e.editorInfo && !e.editorInfo.isSuggesting;
switch (e.code) {
case 'ArrowUp':
Expand Down
2 changes: 1 addition & 1 deletion src/datascience-ui/native-editor/nativeEditor.less
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
.markdown-editor-area {
position: relative;
width:100%;
padding-right: 10px;
padding-right: 8px;
margin-bottom: 0px;
padding-left: 2px;
padding-top: 2px;
Expand Down

0 comments on commit 21fcb0b

Please sign in to comment.