Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
cce62ac
extract diagram editor toolbar:
mabaasit Jun 17, 2025
a43ed7e
add export modal
mabaasit Jun 17, 2025
39793bd
json export
mabaasit Jun 20, 2025
e47687e
tests
mabaasit Jun 20, 2025
dbc1ca0
close modal
mabaasit Jun 20, 2025
6d8de76
Merge branch 'main' into COMPASS-9448-diagram-json-export
mabaasit Jun 20, 2025
087936c
tests
mabaasit Jun 20, 2025
6c21b63
ensure test run
mabaasit Jun 20, 2025
b7d8dcf
fix toast
mabaasit Jun 20, 2025
7c4c14e
fix electron test
mabaasit Jun 20, 2025
f9bcc51
fix link
mabaasit Jun 20, 2025
b65f4c8
ensure its thrown
mabaasit Jun 23, 2025
adc19a1
asset number of selected collections
mabaasit Jun 23, 2025
6c41226
fix modal styles
mabaasit Jun 23, 2025
86e8a92
Merge branch 'main' into COMPASS-9448-diagram-json-export
mabaasit Jun 23, 2025
7fa0480
return null for tests
mabaasit Jun 24, 2025
8c5c14d
Merge branch 'main' into COMPASS-9448-diagram-json-export
mabaasit Jun 24, 2025
ffef0b7
remove comment
mabaasit Jun 24, 2025
88a227b
export image to png
mabaasit Jun 26, 2025
487a2b9
Merge branch 'main' of https://github.com/mongodb-js/compass into COM…
mabaasit Jun 26, 2025
56aadf0
move container to the export png
mabaasit Jun 26, 2025
42b9c8e
add ocr e2e test
mabaasit Jun 26, 2025
68d00cf
abortable export
mabaasit Jun 27, 2025
7b220ca
Merge branch 'main' of https://github.com/mongodb-js/compass into COM…
mabaasit Jun 27, 2025
8a08691
use package methods
mabaasit Jun 30, 2025
3b16d26
clean up
mabaasit Jun 30, 2025
c035d7a
Merge branch 'main' into COMPASS-9449-export-to-png
mabaasit Jun 30, 2025
cda452d
update diagramming
mabaasit Jul 1, 2025
a47faf8
Merge remote-tracking branch 'origin' into COMPASS-9449-export-to-png
mabaasit Jul 1, 2025
15bf100
npm install
mabaasit Jul 1, 2025
ae8e3c6
npm check
mabaasit Jul 1, 2025
840c078
fix test
mabaasit Jul 2, 2025
4bca5a2
Merge branch 'main' of https://github.com/mongodb-js/compass into COM…
mabaasit Jul 2, 2025
0a9a6e4
update npm
mabaasit Jul 2, 2025
aab9aca
Merge branch 'main' of https://github.com/mongodb-js/compass into COM…
mabaasit Jul 2, 2025
c6bc709
fix spaces issue
mabaasit Jul 2, 2025
010b0c6
fix popup for firefox
mabaasit Jul 2, 2025
b34c1f5
skip on win and lowercase test assertions
mabaasit Jul 3, 2025
6136ff4
cr comments
mabaasit Jul 3, 2025
fd4ee26
Merge branch 'main' of https://github.com/mongodb-js/compass into COM…
mabaasit Jul 3, 2025
065851a
npm install
mabaasit Jul 3, 2025
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
642 changes: 421 additions & 221 deletions package-lock.json

Large diffs are not rendered by default.

6 changes: 4 additions & 2 deletions packages/compass-data-modeling/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,18 @@
"@mongodb-js/compass-logging": "^1.7.4",
"@mongodb-js/compass-telemetry": "^1.10.2",
"@mongodb-js/compass-user-data": "^0.7.4",
"@mongodb-js/compass-utils": "^0.9.2",
"@mongodb-js/compass-workspaces": "^0.44.0",
"@mongodb-js/diagramming": "^1.1.0",
"@mongodb-js/diagramming": "^1.2.0",
"bson": "^6.10.3",
"compass-preferences-model": "^2.43.0",
"html-to-image": "1.11.11",
"lodash": "^4.17.21",
"mongodb": "^6.14.1",
"mongodb-ns": "^2.4.2",
"mongodb-schema": "^12.6.2",
"react": "^17.0.2",
"react-dom": "^17.0.2",
"react-redux": "^8.1.3",
"redux": "^4.2.1",
"redux-thunk": "^2.4.2"
Expand All @@ -90,7 +93,6 @@
"depcheck": "^1.4.1",
"mocha": "^10.2.0",
"nyc": "^15.1.0",
"react-dom": "^17.0.2",
"sinon": "^17.0.1",
"typescript": "^5.0.4",
"xvfb-maybe": "^0.2.1"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useCallback, useState } from 'react';
import React, { useCallback, useEffect, useRef, useState } from 'react';
import {
Button,
css,
Expand All @@ -12,6 +12,8 @@ import {
Radio,
RadioGroup,
spacing,
SpinLoader,
openToast,
} from '@mongodb-js/compass-components';
import {
closeExportModal,
Expand All @@ -21,7 +23,9 @@ import {
import { connect } from 'react-redux';
import type { DataModelingState } from '../store/reducer';
import type { StaticModel } from '../services/data-model-storage';
import { exportToJson } from '../services/export-diagram';
import { exportToJson, exportToPng } from '../services/export-diagram';
import { useDiagram } from '@mongodb-js/diagramming';
import { isCancelError } from '@mongodb-js/compass-utils';

const nbsp = '\u00a0';

Expand Down Expand Up @@ -59,20 +63,68 @@ const ExportDiagramModal = ({
model,
onCloseClick,
}: ExportDiagramModalProps) => {
const [exportFormat, setExportFormat] = useState<'json' | null>(null);

const onExport = useCallback(() => {
if (!exportFormat || !model) {
return;
const [exportFormat, setExportFormat] = useState<'png' | 'json' | null>(null);
const diagram = useDiagram();
const [isExporting, setIsExporting] = useState(false);
const abortControllerRef = useRef<AbortController | null>(null);
useEffect(() => {
const cleanup = () => {
if (abortControllerRef.current) {
abortControllerRef.current.abort();
abortControllerRef.current = null;
}
};
const abortController = new AbortController();
if (isModalOpen) {
abortControllerRef.current = abortController;
} else {
cleanup();
}
exportToJson(diagramLabel, model);
return cleanup;
}, [isModalOpen]);

const onClose = useCallback(() => {
setIsExporting(false);
abortControllerRef.current?.abort();
abortControllerRef.current = null;
onCloseClick();
}, [exportFormat, onCloseClick, model, diagramLabel]);
}, [onCloseClick]);

const onExport = useCallback(async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is way too much logic in the UI here (like if you see that you're keeping abort controllers in render, you probably should start thinking about that), this should really be an action in the store instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, i'll add another slice for export. Or, I can do it as a follow up if that sounds okay to you.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Follow-up sounds good, let's not hold this code in the branch for too long 👍

try {
if (!exportFormat || !model) {
return;
}
setIsExporting(true);
if (exportFormat === 'json') {
exportToJson(diagramLabel, model);
} else if (exportFormat === 'png') {
await exportToPng(
diagramLabel,
diagram,
abortControllerRef.current?.signal
);
}
} catch (error) {
if (isCancelError(error)) {
return;
}
openToast('export-diagram-error', {
variant: 'warning',
title: 'Export failed',
description: `An error occurred while exporting the diagram: ${
(error as Error).message
}`,
});
} finally {
onClose();
}
}, [exportFormat, onClose, model, diagram, diagramLabel]);

return (
<Modal
open={isModalOpen}
setOpen={onCloseClick}
setOpen={onClose}
data-testid="export-diagram-modal"
>
<ModalHeader
Expand All @@ -95,6 +147,17 @@ const ExportDiagramModal = ({
<div className={contentContainerStyles}>
<Label htmlFor="">Select file format:</Label>
<RadioGroup className={contentContainerStyles} value={exportFormat}>
<div className={radioItemStyles}>
<Icon glyph="Diagram2" />
<Radio
checked={exportFormat === 'png'}
value="png"
aria-label="PNG"
onClick={() => setExportFormat('png')}
>
PNG
</Radio>
</div>
<div className={radioItemStyles}>
<Icon glyph="CurlyBraces" />
<Radio
Expand All @@ -114,14 +177,13 @@ const ExportDiagramModal = ({
variant="primary"
onClick={() => void onExport()}
data-testid="export-button"
disabled={!exportFormat || !model}
loadingIndicator={<SpinLoader />}
isLoading={isExporting}
>
Export
</Button>
<Button
variant="default"
onClick={onCloseClick}
data-testid="cancel-button"
>
<Button variant="default" onClick={onClose} data-testid="cancel-button">
Cancel
</Button>
</ModalFooter>
Expand Down
33 changes: 0 additions & 33 deletions packages/compass-data-modeling/src/services/export-diagram.ts

This file was deleted.

168 changes: 168 additions & 0 deletions packages/compass-data-modeling/src/services/export-diagram.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
import React from 'react';
import {
getNodesBounds,
getViewportForBounds,
DiagramProvider,
Diagram,
} from '@mongodb-js/diagramming';
import type { DiagramInstance } from '@mongodb-js/diagramming';
import type { StaticModel } from './data-model-storage';
import ReactDOM from 'react-dom';
import { toPng } from 'html-to-image';
import { rafraf, spacing } from '@mongodb-js/compass-components';
import { raceWithAbort } from '@mongodb-js/compass-utils';

function moveSvgDefsToViewportElement(
container: Element,
targetElement: Element
) {
const markerDef = container.querySelector('svg defs');
if (!markerDef) {
return;
}
const diagramSvgElements = targetElement.querySelectorAll('svg');
diagramSvgElements.forEach((svg) => {
const pathsWithMarkers = svg.querySelectorAll(
'path[marker-end], path[marker-start]'
);
if (pathsWithMarkers.length !== 0) {
const clonedDef = markerDef.cloneNode(true);
svg.insertBefore(clonedDef, svg.firstChild);
}
});
markerDef.remove();
}

export async function exportToPng(
fileName: string,
diagram: DiagramInstance,
signal?: AbortSignal
) {
const dataUri = await raceWithAbort(
getExportPngDataUri(diagram),
signal ?? new AbortController().signal
);
downloadFile(dataUri, fileName);
}

export function getExportPngDataUri(diagram: DiagramInstance): Promise<string> {
return new Promise<string>((resolve, _reject) => {
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why we're going to the Promise resolve/reject here? I'm thinking it could lead to an uncaught exception. Could we instead have the cleanup in a finally or catch block?

We could do await something like the rafraf as well and await the toPng.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What would you recommend in this case? I only want to resolve once dom has been converted to a data uri (and as such show loading state to the user). For clean up, its already in place where we export.

Copy link
Member

Choose a reason for hiding this comment

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

I would recommend async/await here with try/catch. I find it makes the code easier to read through and the error handling less prone to be uncaught.

const bounds = getNodesBounds(diagram.getNodes());

const container = document.createElement('div');
container.setAttribute('data-testid', 'export-diagram-container');
// Push it out of the viewport
container.style.position = 'fixed';
container.style.top = '100vh';
container.style.left = '100vw';
container.style.width = `${bounds.width}px`;
container.style.height = `${bounds.height}px`;
document.body.appendChild(container);

const edges = diagram.getEdges();
const nodes = diagram.getNodes().map((node) => ({
...node,
selected: false, // Dont show selected state (blue border)
}));

const reject = (error: Error) => {
document.body.removeChild(container);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like we're missing clean-up on resolve, this should probably be in a finally block

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah, @Anemy also mentioned same and that time i completely misunderstood this. i added this in 6136ff4 along with other fixes. Regarding useToast hook, i'll do a follow up to remove it from compass-components as its not being used elsewhere.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds good!

i'll do a follow up to remove it from compass-components

Just FYI, I already did in eslint update PR, so no need to worry about this (was in the way of some new issues that update started to show there)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: you don't need it here now that you have it in finally

_reject(error);
};

ReactDOM.render(
<DiagramProvider>
<Diagram
edges={edges}
nodes={nodes}
onlyRenderVisibleElements={false}
/>
</DiagramProvider>,
container,
() => {
rafraf(() => {
// For export we are selecting react-flow__viewport element,
// which contains the export canvas. It excludes diagram
// title, minmap, and other UI elements. However, it also
// excludes the svg defs that are currently outside of this element.
// So, when exporting, we need to include those defs as well so that
// edge markers are exported correctly.
Comment on lines +88 to +89
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it be easier to hide what we don't need instead of doing this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we make this configurable in the diagramming package?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wouldn't it be easier to hide what we don't need instead of doing this?

I'll evaluate this, last time (in poc) i checked this, it did not give me good results.

Can we make this configurable in the diagramming package?

These custom marker are rendered here in the package. I am not sure if i follow what you mean by making it configurable?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, comment somehow attached to the wrong place, should've been this part:

It excludes diagram title, minmap, and other UI elements

We're doing all this to avoid including controls in the diagram screenshot. Can we change diagramming package to allow us to render it with hidden controls? Then you don't need to do anything to target an element that doesn't include everything that you need

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah, yeah that should be possible with the package. but again it depends on if we are able to export the root container (.react-flow) and if not, then maybe we don't need it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried to do this and was not able to get it to export correctly using this approach.

const viewportElement = container.querySelector(
'.react-flow__viewport'
);
Comment on lines +90 to +92
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: instead of relying on arbitrary frames you can probably wait for this element to be in the view (similar to what we do in tests sometimes)

if (!viewportElement) {
return reject(new Error('Diagram element not found'));
}

const transform = getViewportForBounds(
bounds,
bounds.width,
bounds.height,
0.5, // Minimum zoom
2, // Maximum zoom
`${spacing[400]}px` // 16px padding
);

// Moving svg defs to the viewport element
moveSvgDefsToViewportElement(container, viewportElement);
rafraf(() => {
toPng(viewportElement as HTMLElement, {
backgroundColor: '#fff',
pixelRatio: 2,
width: bounds.width,
height: bounds.height,
style: {
width: `${bounds.width}px`,
height: `${bounds.height}px`,
transform: `translate(${transform.x}px, ${transform.y}px) scale(${transform.zoom})`,
},
})
.then(resolve)
.catch(reject)
.finally(() => {
document.body.removeChild(container);
});
});
});
}
);
});
}

export function exportToJson(fileName: string, model: StaticModel) {
const json = getExportJsonFromModel(model);
const blob = new Blob([JSON.stringify(json, null, 2)], {
type: 'application/json',
});
const url = window.URL.createObjectURL(blob);
downloadFile(url, fileName, () => {
window.URL.revokeObjectURL(url);
});
}

export function getExportJsonFromModel({
collections,
relationships,
}: StaticModel) {
return {
collections: Object.fromEntries(
collections.map((collection) => {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { ns, jsonSchema, ...ignoredProps } = collection;
return [ns, { ns, jsonSchema }];
})
),
relationships,
};
}

function downloadFile(uri: string, fileName: string, cleanup?: () => void) {
const link = document.createElement('a');
link.download = fileName;
link.href = uri;
link.click();
setTimeout(() => {
link.remove();
cleanup?.();
}, 0);
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ export async function hideAllVisibleToasts(

const toasts = browser.$(Selectors.LGToastContainer).$$('div');
for (const _toast of toasts) {
// if they all went away at some point, just stop
if (!(await isToastContainerVisible(browser))) {
// if they all went away at some point
// or if that toast is not visible anymore just stop
if (!(await isToastContainerVisible(browser)) || !_toast) {
return;
}

Expand Down
2 changes: 2 additions & 0 deletions packages/compass-e2e-tests/helpers/compass.ts
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,8 @@ export async function startBrowser(
'browser.download.folderList': 2,
'browser.download.manager.showWhenStarting': false,
'browser.helperApps.neverAsk.saveToDisk': '*/*',
// Hide the download (progress) panel
'browser.download.alwaysOpenPanel': false,
},
},
},
Expand Down
1 change: 1 addition & 0 deletions packages/compass-e2e-tests/helpers/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1450,6 +1450,7 @@ export const DataModelUndoButton = 'button[aria-label="Undo"]';
export const DataModelRedoButton = 'button[aria-label="Redo"]';
export const DataModelExportButton = 'button[aria-label="Export"]';
export const DataModelExportModal = '[data-testid="export-diagram-modal"]';
export const DataModelExportPngOption = `${DataModelExportModal} input[aria-label="PNG"]`;
export const DataModelExportJsonOption = `${DataModelExportModal} input[aria-label="JSON"]`;
export const DataModelExportModalConfirmButton =
'[data-testid="export-button"]';
Expand Down
Loading
Loading