Skip to content

fix: clarify import functionality COMPASS-9833 #7649

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.

Sign up for GitHub

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
paula-stacho merged 2 commits into from
Dec 15, 2025
Merged

fix: clarify import functionality COMPASS-9833 #7649

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dae006e
fix: clarify import functionality COMPASS-9833
paula-stacho Dec 12, 2025
3d06df5
.
paula-stacho Dec 12, 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
4 changes: 2 additions & 2 deletions packages/compass-data-modeling/src/components/export-diagram-modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,12 @@ const ExportDiagramModal = ({
<Radio
checked={exportFormat === 'diagram'}
value="diagram"
aria-label="Diagram File"
aria-label="MDM File"
onClick={() => onSelectFormat('diagram')}
size="small"
description="Importable into Compass and Data Explorer so teammates can collaborate."
>
Diagram File
MDM File
</Radio>
</div>
<div className={radioItemStyles}>
Expand Down
44 changes: 27 additions & 17 deletions packages/compass-data-modeling/src/components/import-diagram-button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
Button,
type ButtonProps,
FileSelector,
Tooltip,
} from '@mongodb-js/compass-components';

type ImportDiagramButtonProps = Omit<ButtonProps, 'onClick'> & {
Expand All @@ -14,22 +15,31 @@ export const ImportDiagramButton = ({
...buttonProps
}: ImportDiagramButtonProps) => {
return (
<FileSelector
id="import-diagram-file-input"
data-testid="import-diagram-file-input"
multiple={false}
accept=".mdm"
onSelect={(files) => {
if (files.length === 0) {
return;
}
onImportDiagram(files[0]);
}}
trigger={({ onClick }) => (
<Button {...buttonProps} onClick={onClick}>
Import diagram
</Button>
)}
/>
<Tooltip
trigger={
<span>
Comment on lines +18 to +20
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The wrapping <span> prevents proper keyboard navigation and focus management for the button inside. Consider using the disabled prop pattern or removing the span wrapper to maintain accessibility.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

@paula-stacho paula-stacho Dec 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The span is important, we have two triggers on that btn and it gets messed up otherwise. I checked the keyboard navigation and it works fine

Copy link
Member

@Anemy Anemy Dec 12, 2025

Choose a reason for hiding this comment

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

Would a div element with inline styles work? span is usually for text content.

Copy link
Collaborator Author

@paula-stacho paula-stacho Dec 12, 2025

Choose a reason for hiding this comment

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

It would work the same, sure, but is it for text content? According to mdn,

<span> is very much like a <div> element, but <div> is a block-level element whereas a <span> is an inline-level element.

https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/span

<FileSelector
id="import-diagram-file-input"
data-testid="import-diagram-file-input"
multiple={false}
accept=".mdm"
onSelect={(files) => {
if (files.length === 0) {
return;
}
onImportDiagram(files[0]);
}}
trigger={({ onClick }) => (
<Button {...buttonProps} onClick={onClick}>
Import diagram
</Button>
)}
/>
</span>
}
>
Only MDM files exported from Compass or Atlas Data Explorer can be
imported.
</Tooltip>
);
};
2 changes: 1 addition & 1 deletion packages/compass-e2e-tests/helpers/selectors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1460,7 +1460,7 @@ 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 DataModelExportDiagramOption = `${DataModelExportModal} input[aria-label="Diagram File"]`;
export const DataModelExportDiagramOption = `${DataModelExportModal} input[aria-label="MDM File"]`;
export const DataModelExportModalConfirmButton =
'[data-testid="export-button"]';
export const DataModelsListItem = (diagramName?: string) => {
Expand Down
Loading