Skip to content

Commit

Permalink
[Notebook] hide trashcan during edit; add shortcut keys
Browse files Browse the repository at this point in the history
  • Loading branch information
johnriedel committed Jan 22, 2025
1 parent 1fde0d9 commit 45f8b50
Show file tree
Hide file tree
Showing 5 changed files with 200 additions and 9 deletions.
22 changes: 21 additions & 1 deletion e2e/helper/notebookUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,25 @@ async function dragAndDropEmbed(page, notebookObject) {
*/
async function commitEntry(page) {
//Click the Commit Entry button
await page.locator('.c-ne__save-button > button').click();
await page.getByLabel('Save notebook entry').click();
}

/**
* @private
* @param {import('@playwright/test').Page} page
*/
async function commitEntryViaShortcutKey(page) {
//Click the Commit Entry button
await page.keyboard.press('Control+Enter');
}

/**
* @private
* @param {import('@playwright/test').Page} page
*/
async function cancelEntry(page) {
//Press the Escape key to cancel editing entry
await page.keyboard.press('Escape');
}

/**
Expand Down Expand Up @@ -153,7 +171,9 @@ async function createNotebookEntryAndTags(page, iterations = 1) {

export {
addNotebookEntry,
cancelEntry,
commitEntry,
commitEntryViaShortcutKey,
createNotebookAndEntry,
createNotebookEntryAndTags,
dragAndDropEmbed,
Expand Down
159 changes: 158 additions & 1 deletion e2e/tests/functional/plugins/notebook/notebook.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,133 @@ test.describe('Notebook entry tests', () => {
await page.locator('.c-notebook__drag-area').click();
await expect(page.getByLabel('Notebook Entry Input')).toBeVisible();
await expect(page.getByLabel('Notebook Entry', { exact: true })).toHaveClass(/is-selected/);
await expect(page.getByLabel('Save notebook entry')).toBeVisible();
});

test("A new entry that's being created can be saved via ctrl+enter shortcut", async ({
page
}) => {
// Navigate to the notebook object
await page.goto(notebookObject.url);

// Click .c-notebook__drag-area to create a new notebook entry
await page.locator('.c-notebook__drag-area').click();

//verify visible elements
await expect(page.getByLabel('Notebook Entry Input')).toBeVisible();
await expect(page.getByLabel('Delete this entry')).toBeHidden();
await expect(page.getByLabel('Notebook Entry', { exact: true })).toHaveClass(/is-selected/);
await expect(page.getByLabel('Save notebook entry')).toBeVisible();

//add text to textarea
const testText = 'test text';
await page.getByLabel('Notebook Entry Input').fill(testText);

const textareaVal = await page.getByLabel('Notebook Entry Input').inputValue();
await expect(textareaVal).toBe(testText);

//press "Control+Enter" to save
await nbUtils.commitEntryViaShortcutKey(page);

//verify notebook entry is saved
await expect(page.locator('.c-ne__input')).toBeHidden();
await expect(page.getByLabel('Notebook Entry Input')).toBeHidden();
await expect(page.getByLabel('Save notebook entry')).toBeHidden();

await expect(page.getByLabel('Delete this entry')).toBeVisible();
await expect(page.getByLabel('Notebook Entry Display')).toBeVisible();
await expect(page.getByLabel('Notebook Entry Display')).toContainText(testText);
});

test("A new entry that's being created can be canceled via the escape key", async ({ page }) => {
// Navigate to the notebook object
await page.goto(notebookObject.url);

// Click .c-notebook__drag-area to create a new notebook entry
await page.locator('.c-notebook__drag-area').click();

//verify visible elements
await expect(page.getByLabel('Notebook Entry Input')).toBeVisible();
await expect(page.getByLabel('Delete this entry')).toBeHidden();
await expect(page.getByLabel('Notebook Entry', { exact: true })).toHaveClass(/is-selected/);
await expect(page.getByLabel('Save notebook entry')).toBeVisible();

//add text to textarea
const testText = 'test text';
await page.getByLabel('Notebook Entry Input').fill(testText);

const textareaVal = await page.getByLabel('Notebook Entry Input').inputValue();
await expect(textareaVal).toBe(testText);

//press "Escape" key to cancel
await nbUtils.cancelEntry(page);

//verify notebook entry is gone
await expect(page.locator('.c-ne__input')).toBeHidden();
await expect(page.getByLabel('Notebook Entry Input')).toBeHidden();
await expect(page.getByLabel('Delete this entry')).toBeHidden();
await expect(page.getByLabel('Save notebook entry')).toBeHidden();
});

test("An existing entry that's being edited can be canceled via the escape key", async ({
page
}) => {
// Navigate to the notebook object
await page.goto(notebookObject.url);

const testText = 'test text';
const otherText = 'other text';

//create notebook entry
await nbUtils.enterTextEntry(page, testText);

//verify entry
await expect(page.getByLabel('Notebook Entry', { exact: true })).toBeVisible();
await expect(page.getByLabel('Notebook Entry Display')).toContainText(testText);

//make entry the active selection
await page.getByLabel('Notebook Entry', { exact: true }).click();

//edit entry (make textarea visible for editing)
await page.getByLabel('Notebook Entry Display', { exact: true }).click();
await expect(page.getByLabel('Notebook Entry Input')).toBeVisible();

//edit textarea value
await page.getByLabel('Notebook Entry Input').fill(otherText);
const textareaVal = await page.getByLabel('Notebook Entry Input').inputValue();
await expect(textareaVal).toBe(otherText);

//press "Escape" key
await nbUtils.cancelEntry(page);

//verify entry reverts back to the original value
await expect(page.getByLabel('Notebook Entry Input')).toBeHidden();
await expect(page.getByLabel('Notebook Entry Display')).toContainText(testText);
await expect(page.getByLabel('Notebook Entry Display')).not.toContainText(otherText);
});

test('The trashcan is not visible when notebook entry is in edit mode', async ({ page }) => {
// Navigate to the notebook object
await page.goto(notebookObject.url);

const testText = 'test text';

//create notebook entry
await nbUtils.enterTextEntry(page, testText);

//verify entry
await expect(page.getByLabel('Notebook Entry', { exact: true })).toBeVisible();
await expect(page.getByLabel('Delete this entry', { exact: true })).toBeVisible();

//make entry the active selection
await page.getByLabel('Notebook Entry', { exact: true }).click();

//edit entry (make textarea visible for editing)
await page.getByLabel('Notebook Entry Display', { exact: true }).click();
await expect(page.getByLabel('Notebook Entry Input')).toBeVisible();
await expect(page.getByLabel('Delete this entry', { exact: true })).toBeHidden();
});

test('When an object is dropped into a notebook, a new entry is created and it should be focused', async ({
page
}) => {
Expand Down Expand Up @@ -351,7 +477,38 @@ test.describe('Notebook entry tests', () => {
await expect(embed).toHaveClass(/icon-plot-overlay/);
expect(embedName).toBe(overlayPlot.name);
});
test.fixme('new entries persist through navigation events without save', async ({ page }) => {});

test('new entries persist through navigation events without save', async ({ page }) => {
// Navigate to the notebook object
await page.goto(notebookObject.url);

// Click .c-notebook__drag-area to create a new notebook entry
await page.locator('.c-notebook__drag-area').click();

//verify visible elements
await expect(page.getByLabel('Notebook Entry', { exact: true })).toBeVisible();
await expect(page.getByLabel('Notebook Entry Input')).toBeVisible();

//add text to textarea
const testText = 'test text';
await page.getByLabel('Notebook Entry Input').fill(testText);

const textareaVal = await page.getByLabel('Notebook Entry Input').inputValue();
await expect(textareaVal).toBe(testText);

//navigate away from notebook without saving the new notebook entry
await page.getByLabel('Navigate up to parent').click();
await page.goto('./', { waitUntil: 'domcontentloaded' });

await expect(page.getByLabel('Notebook Entry', { exact: true })).toBeHidden();

//navigate back to notebook
await page.goto(notebookObject.url);

await expect(page.getByLabel('Notebook Entry', { exact: true })).toBeVisible();
await expect(page.getByLabel('Notebook Entry Display')).toContainText(testText);
});

test('previous and new entries can be deleted', async ({ page }) => {
// Navigate to the notebook object
await page.goto(notebookObject.url);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ test.describe('Snapshot image tests', () => {
}, fileData);

await page.dispatchEvent('.c-notebook__drag-area', 'drop', { dataTransfer: dropTransfer });
await page.locator('.c-ne__save-button > button').click();
await page.getByLabel('Save notebook entry').click();
// be sure that entry was created
await expect(page.getByText('favicon-96x96.png')).toBeVisible();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ test.describe('Tagging in Notebooks @addInit', () => {

await page.locator('text=To start a new entry, click here or drag and drop any object').click();
await page.getByLabel('Notebook Entry Input').fill(`An entry without tags`);
await page.locator('.c-ne__save-button > button').click();
await page.getByLabel('Save notebook entry').click();

await page.hover('[aria-label="Notebook Entry Display"] >> nth=1');
await page.locator('button[title="Delete this entry"]').last().click();
Expand Down
24 changes: 19 additions & 5 deletions src/plugins/notebook/components/NotebookEntry.vue
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
}}
</span>
</div>
<span v-if="!readOnly && !isLocked" class="c-ne__local-controls--hidden">
<span v-if="!readOnly && !isLocked && !editMode" class="c-ne__local-controls--hidden">
<button
class="c-ne__remove c-icon-button c-icon-button--major icon-trash"
title="Delete this entry"
Expand Down Expand Up @@ -80,15 +80,21 @@
v-else
:id="entry.id"
ref="entryInput"
v-model="entry.text"
v-model="entryTextVal"
class="c-ne__input"
aria-label="Notebook Entry Input"
tabindex="-1"
@mouseleave="canEdit = true"
@keydown.esc="cancelEntry()"
@keydown.ctrl.enter="updateEntryValue($event)"
@blur="updateEntryValue($event)"
></textarea>
<div v-if="editMode" class="c-ne__save-button">
<button class="c-button c-button--major icon-check"></button>
<button
class="c-button c-button--major icon-check"
title="Save notebook entry (Ctrl-Enter)"
aria-label="Save notebook entry"
></button>
</div>
</template>

Expand Down Expand Up @@ -271,6 +277,7 @@ export default {
}
},
emits: [
'cancel-edit',
'delete-entry',
'change-section-page',
'update-entry',
Expand All @@ -283,7 +290,8 @@ export default {
editMode: false,
canEdit: true,
enableEmbedsWrapperScroll: false,
urlWhitelist: []
urlWhitelist: [],
entryTextVal: null
};
},
computed: {
Expand Down Expand Up @@ -347,6 +355,7 @@ export default {
this.renderer = new this.marked.Renderer();
},
mounted() {
this.entryTextVal = this.entry.text;
const originalLinkRenderer = this.renderer.link;
this.renderer.link = this.validateLink.bind(this, originalLinkRenderer);

Expand Down Expand Up @@ -492,6 +501,11 @@ export default {
this.canEdit = false;
}
},
cancelEntry() {
this.editMode = false;
this.entryTextVal = this.entry.text;
this.$emit('cancel-edit');

Check warning on line 507 in src/plugins/notebook/components/NotebookEntry.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/notebook/components/NotebookEntry.vue#L505-L507

Added lines #L505 - L507 were not covered by tests
},
deleteEntry() {
this.$emit('delete-entry', this.entry.id);
},
Expand Down Expand Up @@ -657,7 +671,7 @@ export default {
},
updateEntryValue($event) {
this.editMode = false;
const rawEntryValue = $event.target.value;
const rawEntryValue = this.entryTextVal;

Check warning on line 674 in src/plugins/notebook/components/NotebookEntry.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/notebook/components/NotebookEntry.vue#L674

Added line #L674 was not covered by tests
const sanitizeInput = sanitizeHtml(rawEntryValue, { allowedAttributes: [], allowedTags: [] });
// change &gt back to > for markdown to do blockquotes
const restoredQuoteBrackets = sanitizeInput.replace(/&gt;/g, '>');
Expand Down

0 comments on commit 45f8b50

Please sign in to comment.