Skip to content

Conversation

@paula-stacho
Copy link
Collaborator

Description

As per figma designs: https://www.figma.com/design/nz84lFaVMLqq9ih4ZJtuP6/Data-modeling-viz-tool?node-id=4184-55423&m=dev

Checklist

  • New tests and/or benchmarks are included
  • Documentation is changed or added
  • If this change updates the UI, screenshots/videos are added and a design review is requested
  • If this change could impact the load on the MongoDB cluster, please describe the expected and worst case impact
  • I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Open Questions

Dependents

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

Copilot AI review requested due to automatic review settings December 12, 2025 19:39
@paula-stacho paula-stacho requested a review from a team as a code owner December 12, 2025 19:39
@paula-stacho paula-stacho requested a review from Anemy December 12, 2025 19:39
@github-actions github-actions bot added the fix label Dec 12, 2025
@github-actions github-actions bot added the fix label Dec 12, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the naming of the diagram export format from "Diagram File" to "MDM File" to align with Figma designs. The change improves clarity by using the more specific MDM (MongoDB Data Model) file type designation throughout the UI and test selectors.

  • Updated export modal UI labels and aria-labels from "Diagram File" to "MDM File"
  • Updated E2E test selector to match the new aria-label
  • Added tooltip to import button clarifying that only MDM files from Compass or Atlas Data Explorer can be imported

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
packages/compass-e2e-tests/helpers/selectors.ts Updated test selector to match new "MDM File" aria-label
packages/compass-data-modeling/src/components/import-diagram-button.tsx Added tooltip explaining MDM file import restrictions
packages/compass-data-modeling/src/components/export-diagram-modal.tsx Changed "Diagram File" labels to "MDM File" in export modal

Comment on lines +18 to +20
<Tooltip
trigger={
<span>
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

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

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

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

@paula-stacho paula-stacho merged commit 008030e into main Dec 15, 2025
83 of 84 checks passed
@paula-stacho paula-stacho deleted the COMPASS-9833 branch December 15, 2025 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants