Embedding images in notebook entries #7048

scottbell · 2023-09-12T13:44:56Z

Closes #6737

Describe your changes:

Allows dropping images onto notebooks and entries.

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?
Is this change backwards compatible? For example, developers won't need to change how they are calling the API or how they've extended core plugins such as Tables or Plots.

Author Checklist

Changes address original issue?
Tests included and/or updated with changes?
Command line build passes?
Has this been smoke tested?
Testing instructions included in associated issue OR is this a dependency/testcase change?

Reviewer Checklist

Changes appear to address issue?
Reviewer has tested changes by following the provided instructions?
Changes appear not to be breaking changes?
Appropriate automated tests included?
Code style and in-line documentation are appropriate?
Has associated issue been labelled unverified? (only applicable if this PR closes the issue)
Has associated issue been labelled bug? (only applicable if this PR is for a bug fix)

…ing-images-in-notebook-entries

…entries too

scottbell · 2023-09-12T13:47:36Z

The new feature in action:

first-draft.mov

deploysentinel · 2023-09-12T13:49:31Z

Current Playwright Test Results Summary

✅ 14 Passing

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 09/18/2023 07:57:26am UTC)

Run Details

Running Workflow e2e-couchdb on Github Actions

Commit: d6129c2

Started: 09/18/2023 07:52:26am UTC

View Detailed Build Results

Current Playwright Test Results Summary

✅ 139 Passing - ⚠️ 3 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 09/18/2023 07:57:26am UTC)

Run Details

Running Job e2e-stable on CircleCI

Commit: d6129c2

Started: 09/18/2023 07:50:31am UTC

⚠️ Flakes

📄 functional/plugins/telemetryTable/telemetryTable.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Telemetry Table unpauses and filters data when paused by button and user changes bounds Retry 1 • Initial Attempt	2.74% (2) 2 / 73 runs failed over last 7 days	23.29% (17) 17 / 73 runs flaked over last 7 days

📄 functional/planning/timelist.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Time List Create a Time List, add a single Plan to it and verify all the activities are displayed with no milliseconds Retry 1 • Initial Attempt	0.94% (1) 1 / 106 run failed over last 7 days	49.06% (52) 52 / 106 runs flaked over last 7 days

📄 functional/plugins/notebook/notebookSnapshots.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Snapshot image tests Can drop an image onto a notebook and create a new entry Retry 1 • Initial Attempt	0% (0) 0 / 7 runs failed over last 7 days	28.57% (2) 2 / 7 runs flaked over last 7 days

View Detailed Build Results

codecov · 2023-09-12T14:50:04Z

Codecov Report

Merging #7048 (d6129c2) into master (c7b5ecb) will decrease coverage by 1.29%.
The diff coverage is 57.69%.

@@            Coverage Diff             @@
##           master    #7048      +/-   ##
==========================================
- Coverage   55.58%   54.30%   -1.29%     
==========================================
  Files         650      650              
  Lines       25922    25972      +50     
  Branches     2532     2539       +7     
==========================================
- Hits        14409    14104     -305     
- Misses      10819    11177     +358     
+ Partials      694      691       -3

Flag	Coverage Δ		*Carryforward flag
e2e-full	`42.11% <ø> (+0.10%)`	⬆️	Carriedforward from 3c7a190
e2e-stable	`53.91% <93.75%> (-3.18%)`	⬇️
unit	`48.61% <0.00%> (-0.67%)`	⬇️

*This pull request uses carry forward flags. Click here to find out more.

Files Changed	Coverage Δ
src/plugins/notebook/components/Notebook.vue	`18.42% <0.00%> (-0.68%)`	⬇️
src/plugins/notebook/components/NotebookEmbed.vue	`0.00% <0.00%> (ø)`
src/plugins/notebook/components/NotebookEntry.vue	`15.38% <0.00%> (-1.94%)`	⬇️
src/plugins/notebook/utils/notebook-migration.js	`64.00% <0.00%> (ø)`
src/plugins/notebook/utils/painterroInstance.js	`3.70% <0.00%> (ø)`
src/plugins/notebook/utils/notebook-entries.js	`84.55% <100.00%> (+3.36%)`	⬆️
src/plugins/notebook/utils/notebook-image.js	`51.85% <100.00%> (+39.73%)`	⬆️

... and 55 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7b5ecb...d6129c2. Read the comment docs.

…entries

ozyx

This is really great stuff, nice job! I've got a few suggestions, mostly minor things. We should come up with some more test scenarios for this as well.

src/plugins/notebook/components/Notebook.vue

src/plugins/notebook/components/NotebookEmbed.vue

src/plugins/notebook/utils/notebook-entries.js

src/plugins/notebook/utils/notebook-image.js

ozyx · 2023-09-13T18:21:02Z

src/plugins/notebook/utils/notebook-image.js

-  });
+export async function saveNotebookImageDomainObject(openmct, object) {
+  console.debug('🍎 Saving notebook image domain object', object);
+  await openmct.objects.save(object);


could this cause issues if one user is editing an entry (has an open transaction) and the other is adding an image to an entry? we should test that

I think this is working ok:

editing.entry.and.adding.image.mov

e2e/tests/functional/plugins/notebook/notebookSnapshots.e2e.spec.js

…entries

ozyx

LGTM. Great work~!

scottbell added 8 commits August 25, 2023 16:45

initial drag drop, wip

16bc2c6

Merge remote-tracking branch 'origin/master' into 6737-support-embedd…

66ff2b6

…ing-images-in-notebook-entries

Merge remote-tracking branch 'origin/master' into 6737-support-embedd…

38fc0b9

…ing-images-in-notebook-entries

Merge remote-tracking branch 'origin/master' into 6737-support-embedd…

92cc739

…ing-images-in-notebook-entries

images work as snapshots, but need to disable navigate to actions

bfdcf85

embed image name

00b93e8

works now with images, need to be refactor so can duplicate code for …

4d3447d

…entries too

works dropping on entries too

b63fc60

scottbell linked an issue Sep 12, 2023 that may be closed by this pull request

Support embedding images in notebook entries #6737

Closed

scottbell requested a review from ozyx September 12, 2023 13:45

handle remote images too

3f7794b

add e2e test

68a6dea

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Sep 12, 2023

spelling

ce3d48c

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Sep 12, 2023

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Sep 12, 2023

scottbell marked this pull request as ready for review September 12, 2023 18:46

scottbell changed the title ~~6737 support embedding images in notebook entries~~ Embedding images in notebook entries Sep 12, 2023

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Sep 12, 2023

Merge branch 'master' into 6737-support-embedding-images-in-notebook-…

3f999be

…entries

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Sep 13, 2023

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Sep 13, 2023

ozyx suggested changes Sep 13, 2023

View reviewed changes

address most PR comments

f0a0479

scottbell requested a review from ozyx September 14, 2023 10:11

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Sep 14, 2023

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Sep 14, 2023

Merge branch 'master' into 6737-support-embedding-images-in-notebook-…

3c7a190

…entries

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Sep 15, 2023

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Sep 15, 2023

Merge branch 'master' into 6737-support-embedding-images-in-notebook-…

d6129c2

…entries

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Sep 18, 2023

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Sep 18, 2023

ozyx approved these changes Sep 18, 2023

View reviewed changes

ozyx added this to the Target:3.1.0 milestone Sep 18, 2023

ozyx merged commit 541a022 into master Sep 18, 2023

ozyx deleted the 6737-support-embedding-images-in-notebook-entries branch September 18, 2023 17:56

unlikelyzero added the type:feature Feature. Required intentional design label Sep 18, 2023

This was referenced Oct 3, 2023

[Notebook] Users should not be limited to plain text in raw notebook entries; newlines should be honored. #3190

Closed

[Snapshots] [Notebook] Users should be able to use clipboard images in their notebook entries #3173

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Embedding images in notebook entries #7048

Embedding images in notebook entries #7048

scottbell commented Sep 12, 2023 •

edited by ozyx

Loading

scottbell commented Sep 12, 2023

deploysentinel bot commented Sep 12, 2023 •

edited

Loading

Test Case Results

Test Case Results

Test Case Results

codecov bot commented Sep 12, 2023 •

edited

Loading

ozyx left a comment

ozyx Sep 13, 2023

scottbell Sep 14, 2023 •

edited

Loading

ozyx left a comment

Embedding images in notebook entries #7048

Embedding images in notebook entries #7048

Conversation

scottbell commented Sep 12, 2023 • edited by ozyx Loading

Describe your changes:

All Submissions:

Author Checklist

Reviewer Checklist

scottbell commented Sep 12, 2023

deploysentinel bot commented Sep 12, 2023 • edited Loading

Current Playwright Test Results Summary

Current Playwright Test Results Summary

⚠️ Flakes

Test Case Results

Test Case Results

Test Case Results

codecov bot commented Sep 12, 2023 • edited Loading

Codecov Report

ozyx left a comment

Choose a reason for hiding this comment

ozyx Sep 13, 2023

Choose a reason for hiding this comment

scottbell Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

ozyx left a comment

Choose a reason for hiding this comment

scottbell commented Sep 12, 2023 •

edited by ozyx

Loading

deploysentinel bot commented Sep 12, 2023 •

edited

Loading

codecov bot commented Sep 12, 2023 •

edited

Loading

scottbell Sep 14, 2023 •

edited

Loading