Skip to content

Commit 180d2ab

Browse files
roblourensroblourens
committed
Reveal top of dragged notebook cell,
fix classes after dragging, remove "insert cell" action
1 parent cac92ff commit 180d2ab

7 files changed

Lines changed: 53 additions & 35 deletions

File tree

‎src/vs/workbench/contrib/notebook/browser/contrib/notebookActions.ts‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -556,16 +556,6 @@ registerAction2(class extends InsertCellCommand {
556556
title: localize('notebookActions.insertCodeCellBelow', "Insert Code Cell Below"),
557557
category: NOTEBOOK_ACTIONS_CATEGORY,
558558
icon: { id: 'codicon/add' },
559-
menu: {
560-
id: MenuId.NotebookCellTitle,
561-
order: CellToolbarOrder.InsertCell,
562-
alt: {
563-
id: INSERT_MARKDOWN_CELL_BELOW_COMMAND_ID,
564-
title: localize('notebookActions.insertMarkdownCellBelow', "Insert Markdown Cell Below"),
565-
icon: { id: 'codicon/add' },
566-
},
567-
when: ContextKeyExpr.equals(NOTEBOOK_EDITABLE_CONTEXT_KEY, true)
568-
},
569559
f1: true
570560
},
571561
CellKind.Code,

‎src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ export interface MarkdownCellLayoutChangeEvent {
7878
export interface ICellViewModel {
7979
readonly model: NotebookCellTextModel;
8080
readonly id: string;
81+
dragging: boolean;
8182
handle: number;
8283
uri: URI;
8384
cellKind: CellKind;

‎src/vs/workbench/contrib/notebook/browser/notebookEditor.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -647,14 +647,18 @@ export class NotebookEditor extends BaseEditor implements INotebookEditor {
647647
}
648648

649649
private async moveCellToIndex(index: number, newIdx: number): Promise<void> {
650-
// console.log(`Move ${index} to ${newIdx}`);
650+
if (index < newIdx) {
651+
// newIdx will be one less after index has been removed
652+
newIdx--;
653+
}
654+
651655
if (!this.notebookViewModel!.moveCellToIdx(index, newIdx, true)) {
652656
return;
653657
}
654658

655659
let r: () => void;
656660
DOM.scheduleAtNextAnimationFrame(() => {
657-
this.list?.revealElementInCenterIfOutsideViewport(this.notebookViewModel!.viewCells[index + 1]);
661+
this.list?.revealElementInView(this.notebookViewModel!.viewCells[newIdx]);
658662
r();
659663
});
660664

‎src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts‎

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import { KeyCode } from 'vs/base/common/keyCodes';
4747
import { domEvent } from 'vs/base/browser/event';
4848
import { tokenizeLineToHTML } from 'vs/editor/common/modes/textToHtmlTokenizer';
4949
import { ITextModel } from 'vs/editor/common/model';
50+
import { BaseCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel';
5051

5152
const $ = DOM.$;
5253

@@ -207,7 +208,6 @@ abstract class AbstractCellRenderer {
207208
container.style.position = 'static';
208209
container.style.height = '22px';
209210
}
210-
211211
}
212212

213213
protected createToolbar(container: HTMLElement): ToolBar {
@@ -257,6 +257,14 @@ abstract class AbstractCellRenderer {
257257
updateActions();
258258
}));
259259
}
260+
261+
protected commonRenderElement(element: BaseCellViewModel, index: number, templateData: BaseCellRenderTemplate): void {
262+
if (element.dragging) {
263+
templateData.container.classList.add(DRAGGING_CLASS);
264+
} else {
265+
templateData.container.classList.remove(DRAGGING_CLASS);
266+
}
267+
}
260268
}
261269

262270
export class MarkdownCellRenderer extends AbstractCellRenderer implements IListRenderer<MarkdownCellViewModel, MarkdownCellRenderTemplate> {
@@ -326,6 +334,8 @@ export class MarkdownCellRenderer extends AbstractCellRenderer implements IListR
326334

327335

328336
renderElement(element: MarkdownCellViewModel, index: number, templateData: MarkdownCellRenderTemplate, height: number | undefined): void {
337+
this.commonRenderElement(element, index, templateData);
338+
329339
templateData.currentRenderedCell = element;
330340
templateData.editingContainer!.style.display = 'none';
331341
templateData.cellContainer.innerHTML = '';
@@ -393,6 +403,9 @@ export class MarkdownCellRenderer extends AbstractCellRenderer implements IListR
393403
}
394404
}
395405

406+
const DRAGGING_CLASS = 'cell-dragging';
407+
const DRAGOVER_CLASS = 'cell-dragover';
408+
396409
type DragImageProvider = () => HTMLElement;
397410

398411
export class CellDragAndDropController {
@@ -408,9 +421,20 @@ export class CellDragAndDropController {
408421
const container = templateData.container;
409422
const dragHandle = templateData.dragHandle;
410423

424+
const dragCleanup = () => {
425+
if (this.currentDraggedCell) {
426+
this.currentDraggedCell.dragging = false;
427+
this.currentDraggedCell = undefined;
428+
}
429+
};
430+
411431
templateData.disposables.add(domEvent(dragHandle, DOM.EventType.DRAG_END)(() => {
412432
// TODO
413433
(this.notebookEditor.getInnerWebview() as any)!.element.style['pointer-events'] = '';
434+
435+
// Note, templateData may have a different element rendered into it by now
436+
container.classList.remove(DRAGGING_CLASS);
437+
dragCleanup();
414438
}));
415439

416440
templateData.disposables.add(domEvent(dragHandle, DOM.EventType.DRAG_START)(event => {
@@ -420,19 +444,15 @@ export class CellDragAndDropController {
420444
return;
421445
}
422446

423-
this.currentDraggedCell = templateData.currentRenderedCell;
447+
this.currentDraggedCell = templateData.currentRenderedCell!;
448+
this.currentDraggedCell.dragging = true;
424449

425450
const dragImage = dragImageProvider();
426451
container.parentElement!.appendChild(dragImage);
427452
event.dataTransfer.setDragImage(dragImage, 0, 0);
428453
setTimeout(() => container.parentElement!.removeChild(dragImage!), 0); // Comment this out to debug drag image layout
429454

430-
container.classList.add('cell-dragover');
431-
container.classList.add('cell-dragging');
432-
}));
433-
434-
templateData.disposables.add(domEvent(dragHandle, DOM.EventType.DRAG_END)(event => {
435-
container.classList.remove('cell-dragging');
455+
container.classList.add(DRAGGING_CLASS);
436456
}));
437457

438458
templateData.disposables.add(domEvent(container, DOM.EventType.DRAG_OVER)(event => {
@@ -442,18 +462,20 @@ export class CellDragAndDropController {
442462
templateData.disposables.add(domEvent(container, DOM.EventType.DROP)(event => {
443463
event.preventDefault();
444464

445-
this.notebookEditor.moveCell(this.currentDraggedCell!, templateData.currentRenderedCell!, 'above');
446-
container.classList.remove('cell-dragover');
465+
const draggedCell = this.currentDraggedCell!;
466+
dragCleanup();
467+
this.notebookEditor.moveCell(draggedCell, templateData.currentRenderedCell!, 'above');
468+
container.classList.remove(DRAGOVER_CLASS);
447469
}));
448470

449471
templateData.disposables.add(domEvent(container, DOM.EventType.DRAG_ENTER)(event => {
450472
event.preventDefault();
451-
container.classList.add('cell-dragover');
473+
container.classList.add(DRAGOVER_CLASS);
452474
}));
453475

454476
templateData.disposables.add(domEvent(container, DOM.EventType.DRAG_LEAVE)(event => {
455477
if (!event.relatedTarget || !DOM.isAncestor(event.relatedTarget as HTMLElement, container)) {
456-
container.classList.remove('cell-dragover');
478+
container.classList.remove(DRAGOVER_CLASS);
457479
}
458480
}));
459481
}
@@ -702,6 +724,8 @@ export class CodeCellRenderer extends AbstractCellRenderer implements IListRende
702724
}
703725

704726
renderElement(element: CodeCellViewModel, index: number, templateData: CodeCellRenderTemplate, height: number | undefined): void {
727+
this.commonRenderElement(element, index, templateData);
728+
705729
templateData.currentRenderedCell = element;
706730

707731
if (height === undefined) {

‎src/vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,15 @@ export abstract class BaseCellViewModel extends Disposable implements ICellViewM
9898
private _lastDecorationId: number = 0;
9999
protected _textModel?: model.ITextModel;
100100

101+
private _dragging: boolean = false;
102+
get dragging(): boolean {
103+
return this._dragging;
104+
}
105+
106+
set dragging(v: boolean) {
107+
this._dragging = v;
108+
}
109+
101110
constructor(readonly viewType: string, readonly notebookHandle: number, readonly model: NotebookCellTextModel, public id: string) {
102111
super();
103112

‎src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -539,11 +539,6 @@ export class NotebookViewModel extends Disposable implements FoldingRegionDelega
539539
}
540540

541541
moveCellToIdx(index: number, newIdx: number, synchronous: boolean, pushedToUndoStack: boolean = true): boolean {
542-
if (index < newIdx) {
543-
// newIdx will be one less after index has been removed
544-
newIdx--;
545-
}
546-
547542
const viewCell = this.viewCells[index] as CellViewModel;
548543
if (!viewCell) {
549544
return false;

‎src/vs/workbench/contrib/notebook/test/notebookViewModel.test.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,17 +74,12 @@ suite('NotebookViewModel', () => {
7474
assert.equal(viewModel.viewCells[1].getText(), '//b');
7575

7676
viewModel.moveCellToIdx(0, 1, false);
77-
// no-op (move to after this cell?)
78-
assert.equal(viewModel.viewCells[0].getText(), '//a');
79-
assert.equal(viewModel.viewCells[1].getText(), '//b');
80-
81-
viewModel.moveCellToIdx(0, 2, false);
8277
// b, a, c
8378
assert.equal(viewModel.viewCells[0].getText(), '//b');
8479
assert.equal(viewModel.viewCells[1].getText(), '//a');
8580
assert.equal(viewModel.viewCells[2].getText(), '//c');
8681

87-
viewModel.moveCellToIdx(0, 3, false);
82+
viewModel.moveCellToIdx(0, 2, false);
8883
// a, c, b
8984
assert.equal(viewModel.viewCells[0].getText(), '//a');
9085
assert.equal(viewModel.viewCells[1].getText(), '//c');

0 commit comments

Comments
 (0)