feat(data-modeling): export diagram to json COMPASS-9448 #7046
Conversation
|
|
||
| it('renders nothing if step is not EDITING', function () { | ||
| renderDiagramEditorToolbar({ step: 'ANALYSIS_CANCELED' }); | ||
| expect(() => screen.getByTestId('diagram-editor-toolbar')).to.throw; |
There was a problem hiding this comment.
throw needs to be called to assert
| <Radio value="json" aria-label="JSON"> | ||
| <Icon glyph="CurlyBraces" /> | ||
| {nbsp} | ||
| JSON | ||
| </Radio> |
There was a problem hiding this comment.
This doesn't match the designs where the icon is outside of the radio item, not sure if that's a deliberate choice not to follow the designs here. Also the alignments of items inside the label is slightly off, but catches the eye, in the designs everything is middle aligned, not baseline
There was a problem hiding this comment.
I missed it as I was reusing some of the code from poc. I aligned it with the design in 6c41226
There was a problem hiding this comment.
This is how it looks now
| // TODO: Confirm all collections are selected by default (COMPASS-9309) | ||
| // Note: We'll need to change the UI, right now the labels are disconnected from the checkboxes | ||
| await browser.clickVisible(Selectors.CreateDataModelConfirmButton); |
There was a problem hiding this comment.
You can count the checkboxes or check the <number> total collections selected text instead of waiting for this refactor
|
Small detail - the label isn't vertically aligned with the checkbox |
| if (!exportFormat || !model) { | ||
| return; | ||
| } | ||
| exportToJson(diagramLabel, model); |
There was a problem hiding this comment.
Should we try/catch this and show the error to the user? I'm wondering what happens if a user tries to save in a place where they might not have permissions or another error with file system saving that could happen like no space left.
There was a problem hiding this comment.
I think that will be handled by the toasts which we show for file downloads as well.
There was a problem hiding this comment.
That's correct, both success and failure will be handled on the app level (for electron)
| link.click(); | ||
| setTimeout(() => { | ||
| window.URL.revokeObjectURL(url); | ||
| link.remove(); |
There was a problem hiding this comment.
Should we be showing a toast here? Or is that something that's planned in future work?
There was a problem hiding this comment.
We are already showing toasts for all file downloads
There was a problem hiding this comment.
This is using the browser download so it can benefit from the electron listeners we've added for Schema export. Added a section in our dev Q&A doc https://docs.google.com/document/d/1_yVsHyMwst62WsqcXaUkMIG4n5AwOBnqQzKQRR889Ig/edit?tab=t.0#heading=h.gr92v9azxldt
packages/compass-data-modeling/src/components/diagram-editor-toolbar.tsx
Outdated
Show resolved
Hide resolved
…oolbar.tsx Co-authored-by: Sergey Petushkov <[email protected]>
| collections: Object.fromEntries( | ||
| collections.map((collection) => { | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| const { displayPosition, indexes, ...restOfTheCollection } = collection; |
There was a problem hiding this comment.
I wonder if we shouldn't whitelist the props that should be included instead of blacklisting like this. There is also shardKey and other stuff like samples would probably pop up here.
There was a problem hiding this comment.
yeah that is better and i added it in 69e315f
Preview
Export.to.JSON.mov
Changes in toast
Description
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes