Skip to content
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

When dropping an unsupported file onto a notebook entry, tell the user it isnt supported #7115

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,11 @@
"websockets",
"swgs",
"memlab",
"devmode"
"devmode",
"blockquote",
"blockquotes",
"Blockquote",
"Blockquotes"
],
"dictionaries": ["npm", "softwareTerms", "node", "html", "css", "bash", "en_US"],
"ignorePaths": [
Expand Down
2 changes: 1 addition & 1 deletion e2e/helper/notebookUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ async function enterTextEntry(page, text) {
await page.locator(NOTEBOOK_DROP_AREA).click();

// enter text
await page.getByLabel('Notebook Entry Display').last().click();
await page.getByLabel('Notebook Entry Input').last().fill(text);
await commitEntry(page);
}
Expand All @@ -53,6 +52,7 @@ async function dragAndDropEmbed(page, notebookObject) {
await page.click('button[title="Show selected item in tree"]');
// Drag and drop the SWG into the notebook
await page.dragAndDrop(`text=${swg.name}`, NOTEBOOK_DROP_AREA);
await commitEntry(page);
}

/**
Expand Down
21 changes: 17 additions & 4 deletions e2e/tests/functional/plugins/notebook/notebook.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ test.describe('Notebook entry tests', () => {

// Click .c-notebook__drag-area
await page.locator('.c-notebook__drag-area').click();
await expect(page.getByLabel('Notebook Entry Display')).toBeVisible();
await expect(page.getByLabel('Notebook Entry Input')).toBeVisible();
await expect(page.getByLabel('Notebook Entry', { exact: true })).toHaveClass(/is-selected/);
});
test('When an object is dropped into a notebook, a new entry is created and it should be focused @unstable', async ({
Expand Down Expand Up @@ -514,10 +514,23 @@ test.describe('Notebook entry tests', () => {
const childItem = page.locator('li:has-text("List Item 2") ol li:has-text("Order 2")');
await expect(childItem).toBeVisible();

// Blocks
const blockTest = '```javascript\nconst foo = "bar";\nconst bar = "foo";\n```';
await nbUtils.enterTextEntry(page, blockTest);
// Code Blocks
ozyx marked this conversation as resolved.
Show resolved Hide resolved
const codeblockTest = '```javascript\nconst foo = "bar";\nconst bar = "foo";\n```';
await nbUtils.enterTextEntry(page, codeblockTest);
const codeBlock = page.locator('code.language-javascript:has-text("const foo = \\"bar\\";")');
await expect(codeBlock).toBeVisible();

// Blockquotes
const blockquoteTest =
'This is a quote by Mark Twain:\n> "The man with a new idea is a crank\n>until the idea succeeds."';
await nbUtils.enterTextEntry(page, blockquoteTest);
const firstLineOfBlockquoteText = page.locator(
'blockquote:has-text("The man with a new idea is a crank")'
);
await expect(firstLineOfBlockquoteText).toBeVisible();
const secondLineOfBlockquoteText = page.locator(
'blockquote:has-text("until the idea succeeds")'
);
await expect(secondLineOfBlockquoteText).toBeVisible();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,11 @@ 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();
// be sure that entry was created
await expect(page.getByText('favicon-96x96.png')).toBeVisible();

await page.getByRole('img', { name: 'favicon-96x96.png thumbnail' }).click();

// expect large image to be displayed
await expect(page.getByRole('dialog').getByText('favicon-96x96.png')).toBeVisible();

Expand All @@ -215,3 +214,59 @@ test.describe('Snapshot image tests', () => {
expect(await page.getByRole('img', { name: 'favicon-96x96.png thumbnail' }).count()).toBe(1);
});
});

test.describe('Snapshot image failure tests', () => {
test.use({ failOnConsoleError: false });
test.beforeEach(async ({ page }) => {
//Navigate to baseURL
await page.goto('./', { waitUntil: 'domcontentloaded' });

// Create Notebook
await createDomainObjectWithDefaults(page, {
type: NOTEBOOK_NAME
});
});

test('Get an error notification when dropping unknown file onto notebook entry', async ({
page
}) => {
// fill Uint8Array array with some garbage data
const garbageData = new Uint8Array(100);
const fileData = Array.from(garbageData);

const dropTransfer = await page.evaluateHandle((data) => {
const dataTransfer = new DataTransfer();
const file = new File([new Uint8Array(data)], 'someGarbage.foo', { type: 'unknown/garbage' });
dataTransfer.items.add(file);
return dataTransfer;
}, fileData);

await page.dispatchEvent('.c-notebook__drag-area', 'drop', { dataTransfer: dropTransfer });

// should have gotten a notification from OpenMCT that we couldn't add it
await expect(page.getByText('Unknown object(s) dropped and cannot embed')).toBeVisible();
});

test('Get an error notification when dropping big files onto notebook entry', async ({
page
}) => {
const garbageSize = 15 * 1024 * 1024; // 15 megabytes

await page.addScriptTag({
// make the garbage client side
content: `window.bigGarbageData = new Uint8Array(${garbageSize})`
});

const bigDropTransfer = await page.evaluateHandle(() => {
const dataTransfer = new DataTransfer();
const file = new File([window.bigGarbageData], 'bigBoy.png', { type: 'image/png' });
dataTransfer.items.add(file);
return dataTransfer;
});

await page.dispatchEvent('.c-notebook__drag-area', 'drop', { dataTransfer: bigDropTransfer });

// should have gotten a notification from OpenMCT that we couldn't add it as it's too big
await expect(page.getByText('unable to embed')).toBeVisible();
});
});
1 change: 0 additions & 1 deletion e2e/tests/functional/plugins/notebook/tags.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ test.describe('Tagging in Notebooks @addInit', () => {
await createNotebookEntryAndTags(page);

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ test.describe('Performance tests', () => {
await page.evaluate(() => window.performance.mark('new-notebook-entry-created'));

// Enter Notebook Entry text
await page.getByLabel('Notebook Entry').last().click();
await page.getByLabel('Notebook Entry Input').last().fill('New Entry');
await page.locator('.c-ne__save-button').click();
await page.evaluate(() => window.performance.mark('new-notebook-entry-filled'));
Expand Down
55 changes: 40 additions & 15 deletions src/plugins/notebook/components/NotebookComponent.vue
Original file line number Diff line number Diff line change
Expand Up @@ -625,29 +625,43 @@
dropEvent.preventDefault();
dropEvent.stopImmediatePropagation();

const localImageDropped = dropEvent.dataTransfer.files?.[0]?.type.includes('image');
const imageUrl = dropEvent.dataTransfer.getData('URL');
const dataTransferFiles = Array.from(dropEvent.dataTransfer.files);
const localImageDropped = dataTransferFiles.some((file) => file.type.includes('image'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome refactor

const snapshotId = dropEvent.dataTransfer.getData('openmct/snapshot/id');
const domainObjectData = dropEvent.dataTransfer.getData('openmct/domain-object-path');

Check warning on line 631 in src/plugins/notebook/components/NotebookComponent.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/notebook/components/NotebookComponent.vue#L631

Added line #L631 was not covered by tests
const imageUrl = dropEvent.dataTransfer.getData('URL');
if (localImageDropped) {
// local image dropped from disk (file)
const imageData = dropEvent.dataTransfer.files[0];
const imageEmbed = await createNewImageEmbed(imageData, this.openmct, imageData?.name);
this.newEntry(imageEmbed);
// local image(s) dropped from disk (file)
const embeds = [];
await Promise.all(
dataTransferFiles.map(async (file) => {
if (file.type.includes('image')) {
const imageData = file;
const imageEmbed = await createNewImageEmbed(

Check warning on line 640 in src/plugins/notebook/components/NotebookComponent.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/notebook/components/NotebookComponent.vue#L638-L640

Added lines #L638 - L640 were not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good candidate for a refactor with composables! foreshadowing intensifies

imageData,
this.openmct,

Check warning on line 642 in src/plugins/notebook/components/NotebookComponent.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/notebook/components/NotebookComponent.vue#L642

Added line #L642 was not covered by tests
imageData?.name
);
embeds.push(imageEmbed);
}
})
);
this.newEntry(embeds);
} else if (imageUrl) {
// remote image dropped (URL)
try {
const response = await fetch(imageUrl);
const imageData = await response.blob();
const imageEmbed = await createNewImageEmbed(imageData, this.openmct);
this.newEntry(imageEmbed);
this.newEntry([imageEmbed]);
} catch (error) {
this.openmct.notifications.alert(`Unable to add image: ${error.message} `);
console.error(`Problem embedding remote image`, error);
}
} else if (snapshotId.length) {
// snapshot object
const snapshot = this.snapshotContainer.getSnapshot(snapshotId);
this.newEntry(snapshot.embedObject);
this.newEntry([snapshot.embedObject]);
this.snapshotContainer.removeSnapshot(snapshotId);

const namespace = this.domainObject.identifier.namespace;
Expand All @@ -656,10 +670,9 @@
namespace
);
saveNotebookImageDomainObject(this.openmct, notebookImageDomainObject);
} else {
} else if (domainObjectData) {

Check warning on line 673 in src/plugins/notebook/components/NotebookComponent.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/notebook/components/NotebookComponent.vue#L673

Added line #L673 was not covered by tests
// plain domain object
const data = dropEvent.dataTransfer.getData('openmct/domain-object-path');
const objectPath = JSON.parse(data);
const objectPath = JSON.parse(domainObjectData);

Check warning on line 675 in src/plugins/notebook/components/NotebookComponent.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/notebook/components/NotebookComponent.vue#L675

Added line #L675 was not covered by tests
const bounds = this.openmct.time.bounds();
const snapshotMeta = {
bounds,
Expand All @@ -668,8 +681,15 @@
openmct: this.openmct
};
const embed = await createNewEmbed(snapshotMeta);

this.newEntry(embed);
this.newEntry([embed]);
} else {

Check warning on line 685 in src/plugins/notebook/components/NotebookComponent.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/notebook/components/NotebookComponent.vue#L685

Added line #L685 was not covered by tests
this.openmct.notifications.error(
`Unknown object(s) dropped and cannot embed. Try again with an image or domain object.`
);
console.warn(

Check warning on line 689 in src/plugins/notebook/components/NotebookComponent.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/notebook/components/NotebookComponent.vue#L689

Added line #L689 was not covered by tests
`Unknown object(s) dropped and cannot embed. Try again with an image or domain object.`
);
return;
}
},
focusOnEntryId() {
Expand Down Expand Up @@ -838,12 +858,12 @@
getSelectedSectionId() {
return this.selectedSection?.id;
},
async newEntry(embed, event) {
async newEntry(embeds, event) {
this.startTransaction();
this.resetSearch();
const notebookStorage = this.createNotebookStorageObject();
this.updateDefaultNotebook(notebookStorage);
const id = await addNotebookEntry(this.openmct, this.domainObject, notebookStorage, embed);
const id = await addNotebookEntry(this.openmct, this.domainObject, notebookStorage, embeds);

const element = this.$refs.notebookEntries.querySelector(`#${id}`);
const entryAnnotations = this.notebookAnnotations[id] ?? {};
Expand All @@ -861,6 +881,11 @@
this.filterAndSortEntries();
this.focusEntryId = id;
this.selectedEntryId = id;

// put entry into edit mode

Check warning on line 885 in src/plugins/notebook/components/NotebookComponent.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/notebook/components/NotebookComponent.vue#L884-L885

Added lines #L884 - L885 were not covered by tests
this.$nextTick(() => {
element.dispatchEvent(new Event('click'));

Check warning on line 887 in src/plugins/notebook/components/NotebookComponent.vue

View check run for this annotation

Codecov / codecov/patch

src/plugins/notebook/components/NotebookComponent.vue#L887

Added line #L887 was not covered by tests
});
},
orientationChange() {
this.formatSidebar();
Expand Down
Loading
Loading