From 91645072293b52262b16bd623a02c19bb64e3fd7 Mon Sep 17 00:00:00 2001 From: Sahitya Buddharaju Date: Mon, 15 Jul 2024 18:18:58 -0500 Subject: [PATCH] fix(notebooks): fix UI bugs in notebook --- libs/components/package.json | 8 +- libs/components/src/cell/cell.spec.ts | 11 -- libs/components/src/index.ts | 2 +- .../notebook-cell.scss} | 26 +++-- .../src/notebook-cell/notebook-cell.spec.ts | 11 ++ .../notebook-cell.stories.js} | 8 +- .../notebook-cell.ts} | 35 +++--- libs/components/src/notebook/notebook.scss | 8 +- .../src/notebook/notebook.stories.js | 107 +++++++++++++++++- libs/components/src/notebook/notebook.ts | 104 ++++++++++++----- libs/components/vite.config.js | 2 +- 11 files changed, 240 insertions(+), 82 deletions(-) delete mode 100644 libs/components/src/cell/cell.spec.ts rename libs/components/src/{cell/cell.scss => notebook-cell/notebook-cell.scss} (85%) create mode 100644 libs/components/src/notebook-cell/notebook-cell.spec.ts rename libs/components/src/{cell/cell.stories.js => notebook-cell/notebook-cell.stories.js} (70%) rename libs/components/src/{cell/cell.ts => notebook-cell/notebook-cell.ts} (82%) diff --git a/libs/components/package.json b/libs/components/package.json index 96104efcb8..4f9b11f80d 100644 --- a/libs/components/package.json +++ b/libs/components/package.json @@ -41,10 +41,10 @@ "import": "./button.mjs", "require": "./button.js" }, - "./cell": { - "types": "./cell/cell.d.ts", - "import": "./cell.mjs", - "require": "./cell.js" + "./notebook-cell": { + "types": "./notebook-cell/notebook-cell.d.ts", + "import": "./notebook-cell.mjs", + "require": "./notebook-cell.js" }, "./checkbox": { "types": "./checkbox/checkbox.d.ts", diff --git a/libs/components/src/cell/cell.spec.ts b/libs/components/src/cell/cell.spec.ts deleted file mode 100644 index b42a89825c..0000000000 --- a/libs/components/src/cell/cell.spec.ts +++ /dev/null @@ -1,11 +0,0 @@ -/** - * @vitest-environment jsdom - */ -import { it, describe, expect } from 'vitest'; -import { CovalentCell } from './cell'; - -describe('Cell', () => { - it('should work', () => { - expect(new CovalentCell()).toBeDefined(); - }); -}); diff --git a/libs/components/src/index.ts b/libs/components/src/index.ts index 567ff6b047..d525aa0418 100644 --- a/libs/components/src/index.ts +++ b/libs/components/src/index.ts @@ -3,7 +3,7 @@ export * from './alert/alert'; export * from './app-shell/app-shell'; export * from './badge/badge'; export * from './button/button'; -export * from './cell/cell'; +export * from './notebook-cell/notebook-cell'; export * from './checkbox/checkbox'; export * from './card/card'; export * from './chips/chip'; diff --git a/libs/components/src/cell/cell.scss b/libs/components/src/notebook-cell/notebook-cell.scss similarity index 85% rename from libs/components/src/cell/cell.scss rename to libs/components/src/notebook-cell/notebook-cell.scss index efa7351e7d..63ae2b40ca 100644 --- a/libs/components/src/cell/cell.scss +++ b/libs/components/src/notebook-cell/notebook-cell.scss @@ -27,6 +27,7 @@ cv-code-editor { .selectionIndicator { background-color: var(--cv-theme-primary); border-radius: 2px; + cursor: move; margin-top: 2px; visibility: hidden; min-width: 8px; @@ -34,21 +35,21 @@ cv-code-editor { .timesExecuted { color: var(--cv-theme-outline-variant); + cursor: move; display: inline-flex; font-family: var(--mdc-typography-body2-font-family); font-size: var(--mdc-typography-body2-font-size); + height: 100%; justify-content: end; line-height: var(--mdc-typography-body2-line-height); - padding: 0 0.75rem; - min-width: 8%; - max-width: 8%; + text-wrap: nowrap; .executionCount { box-sizing: border-box; display: inline-block; - max-width: 70%; - padding: 0 2px; + max-width: 32px; overflow: hidden; + padding: 0 2px; text-overflow: ellipsis; white-space: nowrap; } @@ -85,18 +86,21 @@ cv-code-editor { } .cellCodeWrapper { - align-items: flex-start; + align-items: start; display: flex; - justify-content: space-between; - width: calc(100% - 8px); + gap: 0.75rem; + justify-content: end; + width: calc(100% - 2rem); } .cellOutputWrapper { - max-width: 90%; - min-width: 90%; + display: flex; + flex-direction: column; + gap: 0.75rem; + max-width: 92%; + min-width: 92%; } .output { - margin: 0.75rem 0.25rem 0.5rem; max-width: 100%; } diff --git a/libs/components/src/notebook-cell/notebook-cell.spec.ts b/libs/components/src/notebook-cell/notebook-cell.spec.ts new file mode 100644 index 0000000000..975babb06f --- /dev/null +++ b/libs/components/src/notebook-cell/notebook-cell.spec.ts @@ -0,0 +1,11 @@ +/** + * @vitest-environment jsdom + */ +import { it, describe, expect } from 'vitest'; +import { CovalentNotebookCell } from './notebook-cell'; + +describe('Notebook cell', () => { + it('should work', () => { + expect(new CovalentNotebookCell()).toBeDefined(); + }); +}); diff --git a/libs/components/src/cell/cell.stories.js b/libs/components/src/notebook-cell/notebook-cell.stories.js similarity index 70% rename from libs/components/src/cell/cell.stories.js rename to libs/components/src/notebook-cell/notebook-cell.stories.js index 7e70667a80..6762428181 100644 --- a/libs/components/src/cell/cell.stories.js +++ b/libs/components/src/notebook-cell/notebook-cell.stories.js @@ -1,7 +1,7 @@ -import './cell'; +import './notebook-cell'; export default { - title: 'Components/Cell', + title: 'Components/Notebook Cell', args: { code: '', index: 0, @@ -23,10 +23,10 @@ const Template = ({ timesExecuted, }) => { return `
- + >
`; }; diff --git a/libs/components/src/cell/cell.ts b/libs/components/src/notebook-cell/notebook-cell.ts similarity index 82% rename from libs/components/src/cell/cell.ts rename to libs/components/src/notebook-cell/notebook-cell.ts index ba576532de..cffef08de9 100644 --- a/libs/components/src/cell/cell.ts +++ b/libs/components/src/notebook-cell/notebook-cell.ts @@ -1,6 +1,6 @@ import { css, html, LitElement, PropertyValues, unsafeCSS } from 'lit'; import { customElement, property } from 'lit/decorators.js'; -import styles from './cell.scss?inline'; +import styles from './notebook-cell.scss?inline'; import { classMap } from 'lit/directives/class-map.js'; import '../code-editor/code-editor'; @@ -10,7 +10,7 @@ import '../typography/typography'; declare global { interface HTMLElementTagNameMap { - 'cv-cell': CovalentCell; + 'cv-notebook-cell': CovalentNotebookCell; } } @@ -18,8 +18,8 @@ enum CvCellEvents { RUN_CODE = 'cell-run-code', } -@customElement('cv-cell') -export class CovalentCell extends LitElement { +@customElement('cv-notebook-cell') +export class CovalentNotebookCell extends LitElement { /** * The index of the cell in a notebook */ @@ -137,7 +137,7 @@ export class CovalentCell extends LitElement { >${this.code}`; - const output = html` + const output = html`
@@ -152,12 +152,13 @@ export class CovalentCell extends LitElement { renderExecutionCount() { let executionCount = html` `; - if (this.showEditor) { - if (this.loading) { - executionCount = html`*`; - } else if (this.timesExecuted) { - executionCount = html`${this.timesExecuted}`; - } + if (this.language === 'markdown') { + return executionCount; + } + if (this.loading) { + executionCount = html`*`; + } else { + executionCount = html`${this.timesExecuted || ' '}`; } return html`[${executionCount}] :`; } @@ -170,14 +171,18 @@ export class CovalentCell extends LitElement { }; return html`
- +
- ${this.renderExecutionCount()} -
${this.renderEditor()}
+ ${this.renderExecutionCount()} +
+ ${this.renderEditor()} +
`; } } -export default CovalentCell; +export default CovalentNotebookCell; diff --git a/libs/components/src/notebook/notebook.scss b/libs/components/src/notebook/notebook.scss index 4353e139d7..3ebe93c88a 100644 --- a/libs/components/src/notebook/notebook.scss +++ b/libs/components/src/notebook/notebook.scss @@ -2,9 +2,7 @@ --cv-notebook-height: 100%; --cv-notebook-width: 100%; - cv-cell { - cursor: move; - + cv-notebook-cell { &.dragged { transform: translate3d(0, 0, 0); } @@ -16,10 +14,6 @@ } .error { - background-color: var(--cv-theme-negative-8); - border: 1px solid var(--cv-theme-negative-24); - box-sizing: border-box; - color: var(--cv-theme-on-negative-container-20); margin: 0.75rem 1px; padding: 0 0.25rem; } diff --git a/libs/components/src/notebook/notebook.stories.js b/libs/components/src/notebook/notebook.stories.js index 560179b09c..39003ce5d0 100644 --- a/libs/components/src/notebook/notebook.stories.js +++ b/libs/components/src/notebook/notebook.stories.js @@ -1,5 +1,5 @@ import './notebook'; -import '../cell/cell'; +import '../notebook-cell/notebook-cell'; export default { title: 'Components/Notebook', @@ -18,7 +18,110 @@ export default { }, ], errors: ['Error: Could not load data.'], - timesExecuted: 1, + timesExecuted: 12121, + }, + { + code: 'print', + language: 'python', + outputs: [ + { + data: { + 'text/html': ` + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
steptypeamountnameOrigoldbalanceOrignewbalanceOrignameDestoldbalanceDestnewbalanceDestisFraudisFlaggedFraudtxn_id
2PAYMENT1156.11C5490498139044.07887.89M17299582310.00.00037882
1CASH_IN839300.36C8961346134776368.765615669.12C10237140652383291.21412484.090026747
4PAYMENT736.76C19236265231120.0383.24M9106585100.00.00038900
1CASH_OUT598674.03C12721154200.00.0C9859341021184203.57971418.91006647
2CASH_IN111692.26C14753318378014144.648125836.9C1432687668122624.00.00016639
`, + }, + metadata: {}, + output_type: 'display_data', + }, + ], + errors: ['Error: Could not load data.'], + timesExecuted: 0, }, { code: 'select * from table_name;', diff --git a/libs/components/src/notebook/notebook.ts b/libs/components/src/notebook/notebook.ts index 3f039c4439..ad651813e9 100644 --- a/libs/components/src/notebook/notebook.ts +++ b/libs/components/src/notebook/notebook.ts @@ -3,18 +3,21 @@ import { html, LitElement, PropertyValues, + render, TemplateResult, unsafeCSS, } from 'lit'; import { customElement, property } from 'lit/decorators.js'; import markdownit from 'markdown-it'; import styles from './notebook.scss?inline'; -import '../cell/cell'; +import '../alert/alert'; +import '../button/button'; +import '../notebook-cell/notebook-cell'; +import '../code-snippet/code-snippet'; import '../icon/icon'; import '../icon-button/icon-button'; -import '../button/button'; -import '../select/select'; import '../list/list-item'; +import '../select/select'; declare global { interface HTMLElementTagNameMap { @@ -89,8 +92,9 @@ export class CovalentNotebook extends LitElement { // Add a cell to the notebook addCell(cellData: CellData) { - this.cells = [...this.cells, cellData]; - this.requestUpdate(); + this._clipboardCell = { ...cellData }; + this.pasteCell(); + this._clipboardCell = null; } // Copy the selected cell @@ -104,9 +108,7 @@ export class CovalentNotebook extends LitElement { cutCell() { if (this._selectedCellIndex !== null) { this._clipboardCell = this.cells[this._selectedCellIndex]; - this.cells.splice(this._selectedCellIndex, 1); - this._selectedCellIndex = null; - this.requestUpdate(); + this.deleteCell(); } } @@ -115,7 +117,7 @@ export class CovalentNotebook extends LitElement { if (this._selectedCellIndex !== null) { this.cells.splice(this._selectedCellIndex, 1); this._selectedCellIndex = null; - this.requestUpdate(); + this.dispatchUpdatedCells(); } } @@ -131,15 +133,25 @@ export class CovalentNotebook extends LitElement { } if (cell) { this.dispatchEvent( - new CustomEvent(name, { + new CustomEvent('cell-action', { bubbles: true, cancelable: true, - detail: { cell, index: this._selectedCellIndex }, + detail: { cell, index: this._selectedCellIndex, action: name }, }) ); } } + dispatchUpdatedCells(): void { + this.dispatchEvent( + new CustomEvent('cells-updated', { + bubbles: true, + cancelable: true, + detail: { cells: this.cells }, + }) + ); + } + protected firstUpdated(): void { if (this.cells.length) { this.cells.forEach((cell) => { @@ -150,6 +162,27 @@ export class CovalentNotebook extends LitElement { } } + getDragImage(cellIndex: number): HTMLElement { + const cell = this.cells[cellIndex]; + + const template = html` +
+ +
+ `; + const container = document.createElement('div'); + render(template, container); + + return container; + } + // Dispatch an event when the cell type is changed handleCellTypeChange(e: CustomEvent) { const cellType = this.cellTypes[e.detail.index]; @@ -176,6 +209,19 @@ export class CovalentNotebook extends LitElement { target.classList.add('dragged'); e.dataTransfer.effectAllowed = 'move'; e.dataTransfer.setData('text/plain', index.toString()); + + // Get a custom drag image + const dragImageContainer = this.getDragImage(index); + const dragImage = dragImageContainer.firstElementChild as HTMLElement; + dragImage.style.position = 'absolute'; + dragImage.style.top = '-1000px'; // Move off-screen + document.body.appendChild(dragImage); + e.dataTransfer.setDragImage(dragImage, 0, 0); + + // Clean up drag image after drag starts + setTimeout(() => { + document.body.removeChild(dragImage); + }, 0); } } @@ -193,6 +239,7 @@ export class CovalentNotebook extends LitElement { this.handleDragLeave(e); if (this._draggedCellIndex !== null) { const draggedCell = this.cells[this._draggedCellIndex]; + draggedCell.selected = true; this.cells = [ ...this.cells.slice(0, this._draggedCellIndex), ...this.cells.slice(this._draggedCellIndex + 1), @@ -207,13 +254,7 @@ export class CovalentNotebook extends LitElement { this.cells = this.cells.map((cell, idx) => ({ ...cell, index: idx })); this._draggedCellIndex = null; - this.dispatchEvent( - new CustomEvent('drag-finished', { - bubbles: true, - cancelable: true, - detail: { cells: this.cells }, - }) - ); + this.dispatchUpdatedCells(); } } @@ -260,10 +301,12 @@ export class CovalentNotebook extends LitElement { this._selectedCellIndex !== null ? this._selectedCellIndex + 1 : this.cells.length; + this.deselectAllCells(); + this._clipboardCell.selected = true; this.cells.splice(index, 0, { ...this._clipboardCell }); this.cells = this.cells.map((cell, idx) => ({ ...cell, index: idx })); this._selectedCellIndex = index; - this.requestUpdate(); + this.dispatchUpdatedCells(); } } @@ -287,6 +330,7 @@ export class CovalentNotebook extends LitElement { case 'image/png': { const image = document.createElement('img'); image.style.maxWidth = '100%'; + image.draggable = false; image.src = `data:image/png;base64, ${output.data[key]}`; content.appendChild(image); break; @@ -302,7 +346,6 @@ export class CovalentNotebook extends LitElement { protected render() { return html`
- ${this.cells.map( (cell, index) => - html` - ${cell.errors && cell.errors.map((error) => html`${error}`)} - `} - ` + html`
+ ${cell.errors && + cell.errors.map( + (error) => html` + ` + )} +
`} + ` )}
diff --git a/libs/components/vite.config.js b/libs/components/vite.config.js index c328e5a285..0b9b2ad35b 100644 --- a/libs/components/vite.config.js +++ b/libs/components/vite.config.js @@ -15,7 +15,7 @@ module.exports = defineConfig(({ mode }) => { 'libs/components/src/app-shell/app-shell', 'libs/components/src/badge/badge', 'libs/components/src/button/button', - 'libs/components/src/cell/cell', + 'libs/components/src/notebook-cell/notebook-cell', 'libs/components/src/checkbox/checkbox', 'libs/components/src/card/card', 'libs/components/src/chips/chip',