Skip to content

Ensure annotations on empty entries in notebook are not lost #6525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Apr 4, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
c17177a
entries now selected on creation
scottbell Mar 30, 2023
3a00c99
select previous entry on deletion
scottbell Mar 30, 2023
0d7b35b
add deletion test
scottbell Mar 30, 2023
fcb46b0
wip
scottbell Mar 30, 2023
c561f24
fix adding focus selection
scottbell Mar 30, 2023
deb7dc3
remove previous entry selection logic
scottbell Mar 30, 2023
e310039
Merge branch 'master' into 6156-annotations-on-empty-entries-in-noteb…
scottbell Mar 30, 2023
42818c0
Merge branch 'master' into 6156-annotations-on-empty-entries-in-noteb…
scottbell Mar 30, 2023
5e77f9a
Merge branch 'master' into 6156-annotations-on-empty-entries-in-noteb…
scottbell Mar 30, 2023
a785157
Merge branch 'master' into 6156-annotations-on-empty-entries-in-noteb…
scottbell Mar 30, 2023
b7836e5
null check for event
scottbell Mar 30, 2023
e57eedc
Merge branch '6156-annotations-on-empty-entries-in-notebook-are-lost'…
scottbell Mar 30, 2023
96ae28a
address review comments
scottbell Mar 31, 2023
42835a3
address review comments
scottbell Mar 31, 2023
54e082a
Merge branch 'master' into 6156-annotations-on-empty-entries-in-noteb…
scottbell Mar 31, 2023
d768581
refactor tests a bit
scottbell Mar 31, 2023
e8ccd4c
typo
scottbell Mar 31, 2023
aec9ecc
Merge branch 'master' into 6156-annotations-on-empty-entries-in-noteb…
scottbell Mar 31, 2023
44d7c49
resolve conflicts
scottbell Apr 1, 2023
ae8250d
remove clicking on entries
scottbell Apr 3, 2023
78b993e
Merge branch 'master' into 6156-annotations-on-empty-entries-in-noteb…
scottbell Apr 4, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions e2e/tests/functional/plugins/notebook/notebook.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,15 @@ test.describe('Notebook entry tests', () => {
type: NOTEBOOK_NAME
});
});
test.fixme('When a new entry is created, it should be focused', async ({ page }) => {});
test('When a new entry is created, it should be focused and selected', async ({ page }) => {
// Navigate to the notebook object
await page.goto(notebookObject.url);

// Click .c-notebook__drag-area
await page.locator('.c-notebook__drag-area').click();
await expect(page.locator('[aria-label="Notebook Entry Input"]')).toBeVisible();
await expect(page.locator('[aria-label="Notebook Entry"]')).toHaveClass(/is-selected/);
});
test('When an object is dropped into a notebook, a new entry is created and it should be focused @unstable', async ({ page }) => {
// Create Overlay Plot
await createDomainObjectWithDefaults(page, {
Expand Down Expand Up @@ -263,7 +271,25 @@ test.describe('Notebook entry tests', () => {
expect(embedName).toBe('Dropped Overlay Plot');
});
test.fixme('new entries persist through navigation events without save', async ({ page }) => {});
test.fixme('previous and new entries can be deleted', async ({ page }) => {});
test('previous and new entries can be deleted', async ({ page }) => {
// Navigate to the notebook object
await page.goto(notebookObject.url);

await nbUtils.enterTextEntry(page, 'First Entry');
await page.hover('text="First Entry"');
await page.click('button[title="Delete this entry"]');
await page.getByRole('button', { name: 'Ok' }).filter({ hasText: 'Ok' }).click();
await expect(page.locator('text="First Entry"')).toBeHidden();
await nbUtils.enterTextEntry(page, 'Another First Entry');
await nbUtils.enterTextEntry(page, 'Second Entry');
await nbUtils.enterTextEntry(page, 'Third Entry');
await page.hover('[aria-label="Notebook Entry"] >> nth=2');
await page.click('button[title="Delete this entry"] >> nth=2');
await page.getByRole('button', { name: 'Ok' }).filter({ hasText: 'Ok' }).click();
await expect(page.locator('text="Third Entry"')).toBeHidden();
await expect(page.locator('text="Another First Entry"')).toBeVisible();
await expect(page.locator('text="Second Entry"')).toBeVisible();
});
test('when a valid link is entered into a notebook entry, it becomes clickable when viewing', async ({ page }) => {
const TEST_LINK = 'http://www.google.com';

Expand Down
18 changes: 18 additions & 0 deletions e2e/tests/functional/plugins/notebook/tags.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,24 @@ test.describe('Tagging in Notebooks @addInit', () => {
await expect(page.locator('[aria-label="Autocomplete Options"]')).not.toContainText("Driving");
await expect(page.locator('[aria-label="Autocomplete Options"]')).toContainText("Drilling");
});
test('Can add tags with blank entry', async ({ page }) => {
//Go to baseURL
await page.goto('./', { waitUntil: 'networkidle' });

createDomainObjectWithDefaults(page, { type: 'Notebook' });
await selectInspectorTab(page, 'Annotations');

await nbUtils.enterTextEntry(page, '');
await page.hover(`button:has-text("Add Tag")`);
await page.locator(`button:has-text("Add Tag")`).click();

// Click inside the tag search input
await page.locator('[placeholder="Type to select tag"]').click();
// Select the "Driving" tag
await page.locator('[aria-label="Autocomplete Options"] >> text=Driving').click();

await expect(page.locator('[aria-label="Notebook Entry"]')).toContainText("Driving");
});
test('Can cancel adding tags', async ({ page }) => {
await createNotebookAndEntry(page);

Expand Down
22 changes: 18 additions & 4 deletions src/plugins/notebook/components/Notebook.vue
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
v-if="selectedPage && !selectedPage.isLocked"
:class="{ 'disabled': activeTransaction }"
class="c-notebook__drag-area icon-plus"
@click="newEntry()"
@click="newEntry(null, $event)"
@dragover="dragOver"
@drop.capture="dropCapture"
@drop="dropOnEntry($event)"
Expand Down Expand Up @@ -193,7 +193,7 @@ import SearchResults from './SearchResults.vue';
import Sidebar from './Sidebar.vue';
import ProgressBar from '../../../ui/components/ProgressBar.vue';
import { clearDefaultNotebook, getDefaultNotebook, setDefaultNotebook, setDefaultNotebookSectionId, setDefaultNotebookPageId } from '../utils/notebook-storage';
import { addNotebookEntry, createNewEmbed, getEntryPosById, getNotebookEntries, mutateObject } from '../utils/notebook-entries';
import { addNotebookEntry, createNewEmbed, getEntryPosById, getNotebookEntries, mutateObject, selectEntry } from '../utils/notebook-entries';
import { saveNotebookImageDomainObject, updateNamespaceOfDomainObject } from '../utils/notebook-image';
import { isNotebookViewType, RESTRICTED_NOTEBOOK_TYPE } from '../notebook-constants';

Expand Down Expand Up @@ -793,15 +793,29 @@ export default {

return section.id;
},
async newEntry(embed = null) {
async newEntry(embed, event) {
this.startTransaction();
this.resetSearch();
const notebookStorage = this.createNotebookStorageObject();
this.updateDefaultNotebook(notebookStorage);
const id = await addNotebookEntry(this.openmct, this.domainObject, notebookStorage, embed);

const element = this.$refs.notebookEntries.querySelector(`#${id}`);
const entryAnnotations = this.notebookAnnotations[id] ?? {};
selectEntry({
element,
entryId: id,
domainObject: this.domainObject,
openmct: this.openmct,
notebookAnnotations: entryAnnotations
});
if (event) {
event.stopPropagation();
}

this.filterAndSortEntries();
this.focusEntryId = id;
this.selectedEntryId = id;
this.filterAndSortEntries();
},
orientationChange() {
this.formatSidebar();
Expand Down
45 changes: 13 additions & 32 deletions src/plugins/notebook/components/NotebookEntry.vue
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
@dragover="changeCursor"
@drop.capture="cancelEditMode"
@drop.prevent="dropOnEntry"
@click="selectEntry($event, entry)"
@click="selectAndEmitEntry($event, entry)"
>
<div class="c-ne__time-and-content">
<div class="c-ne__time-and-creator-and-delete">
Expand Down Expand Up @@ -164,7 +164,7 @@
<script>
import NotebookEmbed from './NotebookEmbed.vue';
import TextHighlight from '../../../utils/textHighlight/TextHighlight.vue';
import { createNewEmbed } from '../utils/notebook-entries';
import { createNewEmbed, selectEntry } from '../utils/notebook-entries';
import { saveNotebookImageDomainObject, updateNamespaceOfDomainObject } from '../utils/notebook-image';

import sanitizeHtml from 'sanitize-html';
Expand Down Expand Up @@ -479,37 +479,18 @@ export default {
updateEntryValue($event) {
this.editMode = false;
const value = $event.target.innerText;
if (value !== this.entry.text && value.match(/\S/)) {
this.entry.text = sanitizeHtml(value, SANITIZATION_SCHEMA);
this.timestampAndUpdate();
} else {
this.$emit('cancelEdit');
}
this.entry.text = sanitizeHtml(value, SANITIZATION_SCHEMA);
this.timestampAndUpdate();
},
selectEntry(event, entry) {
const targetDetails = {};
const keyString = this.openmct.objects.makeKeyString(this.domainObject.identifier);
targetDetails[keyString] = {
entryId: entry.id
};
const targetDomainObjects = {};
targetDomainObjects[keyString] = this.domainObject;
this.openmct.selection.select(
[
{
element: event.currentTarget,
context: {
type: 'notebook-entry-selection',
item: this.domainObject,
targetDetails,
targetDomainObjects,
annotations: this.notebookAnnotations,
annotationType: this.openmct.annotation.ANNOTATION_TYPES.NOTEBOOK,
onAnnotationChange: this.timestampAndUpdate
}
}
],
false);
selectAndEmitEntry(event, entry) {
selectEntry({
element: event.currentTarget,
entryId: entry.id,
domainObject: this.domainObject,
openmct: this.openmct,
onAnnotationChange: this.timestampAndUpdate,
notebookAnnotations: this.notebookAnnotations
});
event.stopPropagation();
this.$emit('entry-selection', this.entry);
}
Expand Down
29 changes: 29 additions & 0 deletions src/plugins/notebook/utils/notebook-entries.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,35 @@ export function addEntryIntoPage(notebookStorage, entries, entry) {
return newEntries;
}

export function selectEntry({
element, entryId, domainObject, openmct,
onAnnotationChange, notebookAnnotations
}) {
const targetDetails = {};
const keyString = openmct.objects.makeKeyString(domainObject.identifier);
targetDetails[keyString] = {
entryId
};
const targetDomainObjects = {};
targetDomainObjects[keyString] = domainObject;
openmct.selection.select(
[
{
element,
context: {
type: 'notebook-entry-selection',
item: domainObject,
targetDetails,
targetDomainObjects,
annotations: notebookAnnotations,
annotationType: openmct.annotation.ANNOTATION_TYPES.NOTEBOOK,
onAnnotationChange
}
}
],
false);
}

export function getHistoricLinkInFixedMode(openmct, bounds, historicLink) {
if (historicLink.includes('tc.mode=fixed')) {
return historicLink;
Expand Down