Skip to content

Commit 631acd2

Browse files
authored
Add cell button (microsoft#7450)
* Add cell command * Make add and insert consitent * Put add button closer to the spec * Fix run button too * Fix switch to markdown to set focus correctly * Fix markdown focus change * Fix top line to show up when no cells Fix focus issues when arrowing * Add news entry * Use span inside of a button * Add support for setting the __file__ variable. Fixes 5459 * Fixup line on hover * Add bug fix news entry * Add mocks for the new __file__ addition
1 parent e301b34 commit 631acd2

11 files changed

Lines changed: 335 additions & 44 deletions

File tree

‎news/2 Fixes/5459.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Set the \_\_file\_\_ variable whenever running code so that \_\_file\_\_ usage works.

‎news/2 Fixes/7362.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add the 'add cell' line between cells, on cells, and at the bottom and top.

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,11 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
493493
await this.jupyterDebugger.startDebugging(this.notebook);
494494
}
495495

496+
// Set the file variable
497+
if (file !== Identifiers.EmptyFileName) {
498+
await this.notebook.execute(`__file__ = '${file.replace(/\\/g, '\\\\')}'`, file, line, uuid(), undefined, true);
499+
}
500+
496501
// Attempt to evaluate this cell in the jupyter notebook
497502
const observable = this.notebook.executeObservable(code, file, line, id, false);
498503

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
import * as React from 'react';
6+
import { Image, ImageName } from '../react-common/image';
7+
import { getLocString } from '../react-common/locReactSide';
8+
9+
interface IAddCellLineProps {
10+
baseTheme: string;
11+
includePlus: boolean;
12+
className: string;
13+
click() : void;
14+
}
15+
16+
export class AddCellLine extends React.Component<IAddCellLineProps> {
17+
constructor(props: IAddCellLineProps) {
18+
super(props);
19+
}
20+
21+
public render() {
22+
const className = `add-cell-line ${this.props.className}`;
23+
const tooltip = getLocString('DataScience.insertBelow', 'Insert cell below');
24+
const plus = this.props.includePlus ? <Image baseTheme={this.props.baseTheme} class='image-button-image' image={ImageName.InsertBelow} /> : null;
25+
return (
26+
<div className={className}>
27+
<button role='button' aria-pressed='false' title={tooltip} aria-label={tooltip} className='add-cell-line-button' onClick={this.props.click}>
28+
{plus}
29+
<span className='add-cell-line-divider'/>
30+
</button>
31+
</div>
32+
);
33+
}
34+
35+
}

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

Lines changed: 80 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { IKeyboardEvent } from '../react-common/event';
1919
import { Image, ImageName } from '../react-common/image';
2020
import { ImageButton } from '../react-common/imageButton';
2121
import { getLocString } from '../react-common/locReactSide';
22+
import { AddCellLine } from './addCellLine';
2223
import { NativeEditorStateController } from './nativeEditorStateController';
2324

2425
interface INativeCellProps {
@@ -36,6 +37,7 @@ interface INativeCellProps {
3637
selectedCell?: string;
3738
focusedCell?: string;
3839
focusCell(cellId: string, focusCode: boolean): void;
40+
selectCell(cellId: string): void;
3941
}
4042

4143
interface INativeCellState {
@@ -46,6 +48,7 @@ export class NativeCell extends React.Component<INativeCellProps, INativeCellSta
4648
private inputRef: React.RefObject<CellInput> = React.createRef<CellInput>();
4749
private wrapperRef: React.RefObject<HTMLDivElement> = React.createRef<HTMLDivElement>();
4850
private lastKeyPressed: string | undefined;
51+
private pendingFocusLoss?: () => void;
4952

5053
constructor(prop: INativeCellProps) {
5154
super(prop);
@@ -120,6 +123,12 @@ export class NativeCell extends React.Component<INativeCellProps, INativeCellSta
120123
return this.props.cellVM.cell.data.cell_type === 'markdown';
121124
}
122125

126+
private isLastCell = () => {
127+
const cellVMs = this.props.stateController.getState().cellVMs;
128+
const index = cellVMs.indexOf(this.props.cellVM);
129+
return index === cellVMs.length - 1;
130+
}
131+
123132
private isSelected = () => {
124133
return this.props.selectedCell === this.cellId;
125134
}
@@ -145,6 +154,7 @@ export class NativeCell extends React.Component<INativeCellProps, INativeCellSta
145154
{this.renderCollapseBar(false)}
146155
{this.renderOutput()}
147156
</div>
157+
{this.renderAddDivider(false)}
148158
{this.renderMiddleToolbar()}
149159
</div> :
150160
<div className='cell-result-container'>
@@ -153,6 +163,7 @@ export class NativeCell extends React.Component<INativeCellProps, INativeCellSta
153163
{this.renderControls()}
154164
{this.renderInput()}
155165
</div>
166+
{this.renderAddDivider(true)}
156167
{this.renderMiddleToolbar()}
157168
<div className='cell-row-container'>
158169
{this.renderCollapseBar(false)}
@@ -208,7 +219,8 @@ export class NativeCell extends React.Component<INativeCellProps, INativeCellSta
208219

209220
private shouldRenderOutput(): boolean {
210221
if (this.isCodeCell()) {
211-
return this.hasOutput() && this.getCodeCell().outputs && !this.props.hideOutput;
222+
const cell = this.getCodeCell();
223+
return this.hasOutput() && cell.outputs && !this.props.hideOutput && (Array.isArray(cell.outputs) && cell.outputs.length !== 0);
212224
} else if (this.isMarkdownCell()) {
213225
return !this.state.showingMarkdownEditor;
214226
}
@@ -441,29 +453,8 @@ export class NativeCell extends React.Component<INativeCellProps, INativeCellSta
441453
this.props.stateController.sendCommand(NativeCommandType.Run, 'keyboard');
442454
}
443455

444-
private moveSelectionToExisting = (cellId: string) => {
445-
// Cell should already exist in the UI
446-
if (this.wrapperRef) {
447-
const wasFocused = this.isFocused();
448-
this.props.stateController.selectCell(cellId, wasFocused ? cellId : undefined);
449-
this.props.focusCell(cellId, wasFocused ? true : false);
450-
}
451-
}
452-
453456
private moveSelection = (cellId: string) => {
454-
// Check to see that this cell already exists in our window (it's part of the rendered state
455-
const cells = this.getNonMessageCells();
456-
if (!cells || !cells.find(c => c.id === cellId)) {
457-
// Force selection change right now as we don't need the cell to exist
458-
// to make it selected (otherwise we'll get a flash)
459-
const wasFocused = this.isFocused();
460-
this.props.stateController.selectCell(cellId, wasFocused ? cellId : undefined);
461-
462-
// Then wait to give it actual input focus
463-
setTimeout(() => this.moveSelectionToExisting(cellId), 1);
464-
} else {
465-
this.moveSelectionToExisting(cellId);
466-
}
457+
this.props.selectCell(cellId);
467458
}
468459

469460
private submitCell = (possibleContents?: string) => {
@@ -483,6 +474,14 @@ export class NativeCell extends React.Component<INativeCellProps, INativeCellSta
483474
}
484475
}
485476

477+
private addNewCell = () => {
478+
const newCell = this.props.stateController.insertBelow(this.props.cellVM.cell.id, true);
479+
this.props.stateController.sendCommand(NativeCommandType.AddToEnd, 'mouse');
480+
if (newCell) {
481+
this.props.selectCell(newCell);
482+
}
483+
}
484+
486485
private renderNavbar = () => {
487486
const cellId = this.props.cellVM.cell.id;
488487

@@ -496,6 +495,12 @@ export class NativeCell extends React.Component<INativeCellProps, INativeCellSta
496495
};
497496
const canMoveUp = this.props.stateController.canMoveUp(cellId);
498497
const canMoveDown = this.props.stateController.canMoveDown(cellId);
498+
const addButtonRender = this.getNextCellId() !== undefined ?
499+
<div className='navbar-add-button'>
500+
<ImageButton baseTheme={this.props.baseTheme} onClick={this.addNewCell} tooltip={getLocString('DataScience.insertBelow', 'Insert cell below')}>
501+
<Image baseTheme={this.props.baseTheme} class='image-button-image' image={ImageName.InsertBelow} />
502+
</ImageButton>
503+
</div> : null;
499504

500505
return (
501506
<div className='navbar-div'>
@@ -509,13 +514,29 @@ export class NativeCell extends React.Component<INativeCellProps, INativeCellSta
509514
<Image baseTheme={this.props.baseTheme} class='image-button-image' image={ImageName.Down} />
510515
</ImageButton>
511516
</div>
517+
{addButtonRender}
512518
</div>
513519
);
514520
}
521+
522+
private renderAddDivider = (checkOutput: boolean) => {
523+
// Skip on the last cell
524+
if (!this.isLastCell()) {
525+
// Divider should only show if no output
526+
if (!checkOutput || !this.shouldRenderOutput()) {
527+
return (
528+
<AddCellLine className='add-divider' baseTheme={this.props.baseTheme} includePlus={false} click={this.addNewCell} />
529+
);
530+
}
531+
}
532+
533+
return null;
534+
}
535+
515536
private renderMiddleToolbar = () => {
516537
const cellId = this.props.cellVM.cell.id;
517538
const deleteCell = () => {
518-
const cellToSelect = this.getPrevCellId() || this.getNextCellId();
539+
const cellToSelect = this.getNextCellId() || this.getPrevCellId();
519540
this.props.stateController.deleteCell(cellId);
520541
this.props.stateController.sendCommand(NativeCommandType.DeleteCell, 'mouse');
521542
setTimeout(() => {
@@ -536,14 +557,35 @@ export class NativeCell extends React.Component<INativeCellProps, INativeCellSta
536557
const canRunBelow = this.props.cellVM.cell.state === CellState.finished || this.props.cellVM.cell.state === CellState.error;
537558
const switchTooltip = this.props.cellVM.cell.data.cell_type === 'code' ? getLocString('DataScience.switchToMarkdown', 'Change to markdown') :
538559
getLocString('DataScience.switchToCode', 'Change to code');
539-
const switchImage = this.props.cellVM.cell.data.cell_type === 'code' ? ImageName.SwitchToMarkdown : ImageName.SwitchToCode;
540-
const switchCell = this.props.cellVM.cell.data.cell_type === 'code' ? () => {
560+
const switchToMarkdown = () => {
541561
this.props.stateController.changeCellType(cellId, 'markdown');
542562
this.props.stateController.sendCommand(NativeCommandType.ChangeToMarkdown, 'mouse');
543-
} : () => {
544-
this.props.stateController.changeCellType(cellId, 'code');
545-
this.props.stateController.sendCommand(NativeCommandType.ChangeToCode, 'mouse');
563+
setTimeout(() => this.props.focusCell(cellId, true), 10);
546564
};
565+
const switchToCode = () => {
566+
const handler = () => {
567+
setTimeout(() => {
568+
this.props.stateController.changeCellType(cellId, 'code');
569+
this.props.stateController.sendCommand(NativeCommandType.ChangeToCode, 'mouse');
570+
this.props.focusCell(cellId, true);
571+
}, 10);
572+
};
573+
574+
// This is special. Coming in on a mouse down event so we get
575+
// called before focus changes. After focus changes, then switch to code
576+
if (this.state.showingMarkdownEditor) {
577+
this.pendingFocusLoss = handler;
578+
} else {
579+
handler();
580+
}
581+
};
582+
const switchButton = this.props.cellVM.cell.data.cell_type === 'code' ?
583+
<ImageButton baseTheme={this.props.baseTheme} onClick={switchToMarkdown} tooltip={switchTooltip}>
584+
<Image baseTheme={this.props.baseTheme} class='image-button-image' image={ImageName.SwitchToMarkdown} />
585+
</ImageButton> :
586+
<ImageButton baseTheme={this.props.baseTheme} onMouseDown={switchToCode} tooltip={switchTooltip}>
587+
<Image baseTheme={this.props.baseTheme} class='image-button-image' image={ImageName.SwitchToCode} />
588+
</ImageButton>;
547589

548590
return (
549591
<div className='native-editor-celltoolbar-middle'>
@@ -553,9 +595,7 @@ export class NativeCell extends React.Component<INativeCellProps, INativeCellSta
553595
<ImageButton baseTheme={this.props.baseTheme} onClick={runBelow} disabled={!canRunBelow} tooltip={getLocString('DataScience.runBelow', 'Run cell and below')}>
554596
<Image baseTheme={this.props.baseTheme} class='image-button-image' image={ImageName.RunBelow} />
555597
</ImageButton>
556-
<ImageButton baseTheme={this.props.baseTheme} onClick={switchCell} tooltip={switchTooltip}>
557-
<Image baseTheme={this.props.baseTheme} class='image-button-image' image={switchImage} />
558-
</ImageButton>
598+
{switchButton}
559599
<ImageButton baseTheme={this.props.baseTheme} onClick={deleteCell} tooltip={getLocString('DataScience.deleteCell', 'Delete cell')}>
560600
<Image baseTheme={this.props.baseTheme} class='image-button-image' image={ImageName.Delete} />
561601
</ImageButton>
@@ -631,16 +671,21 @@ export class NativeCell extends React.Component<INativeCellProps, INativeCellSta
631671
private onMarkdownUnfocused = () => {
632672
this.props.stateController.codeLostFocus(this.cellId);
633673

674+
// There might be a pending focus loss handler.
675+
if (this.pendingFocusLoss) {
676+
const func = this.pendingFocusLoss;
677+
this.pendingFocusLoss = undefined;
678+
func();
679+
}
680+
634681
// Indicate not showing the editor anymore. The equivalent of this
635682
// is not when we receive focus but when we GIVE focus to the markdown editor
636683
// otherwise we wouldn't be able to display it.
637684
this.setState({showingMarkdownEditor: false});
638685
}
639686

640687
private renderOutput = (): JSX.Element | null => {
641-
const cell = this.props.cellVM.cell.data;
642-
643-
if (this.shouldRenderOutput() && ((cell.cell_type === 'markdown') || (Array.isArray(cell.outputs) && cell.outputs.length !== 0))) {
688+
if (this.shouldRenderOutput()) {
644689
return (
645690
<CellOutput
646691
cellVM={this.props.cellVM}

0 commit comments

Comments
 (0)