Skip to content

feat(compass-data-modeling): add relationship via dragging COMPASS-9332 #7146

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.

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
merged 6 commits into from
Aug 7, 2025

Conversation

paula-stacho
Copy link
Contributor

@paula-stacho paula-stacho commented Jul 25, 2025

Description

E2E tests diagram - drawer interactions will be a follow up, I'll add a note to test this scenario as well

Screen.Recording.2025-07-31.at.16.38.17.mov

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
  • 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)

@github-actions github-actions bot added the feat label Jul 25, 2025
@paula-stacho paula-stacho added the feature flagged PRs labeled with this label will not be included in the release notes of the next release label Jul 25, 2025
active={isInRelationshipDrawingMode}
aria-pressed={isInRelationshipDrawingMode}
>
<Icon glyph="Relationship"></Icon>
Copy link
Member

@Anemy Anemy Jul 26, 2025

Choose a reason for hiding this comment

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

Should we provide a tooltip describing how to use the tool? I'm not sure it's intuitive for a user that after clicking the button then they should drag from one collection to another.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm honestly not even sure that the tooltip will be enough, it takes a moment for those to show up when you hover and we will have to explain the whole thing there, this feels a bit clunky. Maybe there's some other way to do this such as the action is self-evident? Like some visual change in the diagram itself that indicates that the nodes are in some special state?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can also barely see that the icon button is "active". Maybe this should be a toggle instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The active state is a temporary solution, I've discussed this with Ben and he needs some time to think about the toggle.

Copy link
Contributor Author

@paula-stacho paula-stacho Jul 28, 2025

Choose a reason for hiding this comment

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

One can see that the pointer changes on the nodes in this state, I agree it's very subtle, this is exactly what I meant last week about the design concerns on this one. I'm currently thinking of opening this PR anyway (just need to rebase and hoping to resolve one more thing), maybe we'll get some ideas from the internal review.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I know that we're doing the best we can here with what design has given us, but this currently feels very subpar as an experience. If we want to merge this, let's open a ticket for some project to make sure we take care of this, I'm afraid that without this, we will just forget to address this.

@paula-stacho paula-stacho force-pushed the COMPASS-9332 branch 2 times, most recently from b371164 to 511887c Compare July 28, 2025 11:11
@paula-stacho paula-stacho marked this pull request as ready for review July 28, 2025 11:50
@Copilot Copilot AI review requested due to automatic review settings July 28, 2025 11:50
@paula-stacho paula-stacho requested a review from a team as a code owner July 28, 2025 11:50
Copy link
Contributor

@Copilot 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 implements the ability to create relationships between collections by dragging from one node to another in the data modeling diagram. The feature adds a toggle button to enter "relationship drawing mode" where collections become connectable and draggable behavior is disabled to allow for connection drawing.

  • Adds a new relationship drawing mode with toggle button in the toolbar
  • Modifies collection nodes to be connectable when in relationship drawing mode
  • Updates the relationship creation logic to accept both source and target namespaces

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/compass-data-modeling/src/utils/nodes-and-edges.tsx Updates collectionToDiagramNode to accept relationship drawing mode and set node connectable/draggable properties
packages/compass-data-modeling/src/store/diagram.ts Modifies createNewRelationship to accept both local and foreign namespaces
packages/compass-data-modeling/src/store/analysis-process.ts Updates call to collectionToDiagramNode with new object parameter format
packages/compass-data-modeling/src/components/diagram-editor.tsx Adds relationship drawing mode state and connection handling logic
packages/compass-data-modeling/src/components/diagram-editor-toolbar.tsx Adds new relationship toggle button with tooltip
packages/compass-data-modeling/src/components/diagram-editor-toolbar.spec.tsx Adds tests for the new relationship button functionality

@paula-stacho paula-stacho force-pushed the COMPASS-9332 branch 2 times, most recently from 333f3a0 to 4c32197 Compare July 31, 2025 14:37
Copy link
Collaborator

@gribnoysup gribnoysup left a comment

Choose a reason for hiding this comment

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

Code looks good, but just want to make sure: do we have a follow-up ticket for this to improve the UI? These visuals has the same issues I mentioned before: it's really unclear that you toggled the mode and what state you're in

@paula-stacho
Copy link
Contributor Author

Code looks good, but just want to make sure: do we have a follow-up ticket for this to improve the UI? These visuals has the same issues I mentioned before: it's really unclear that you toggled the mode and what state you're in

Added https://jira.mongodb.org/browse/COMPASS-9665 to the polish and also to our QA document, might be something we want to ask users about specifically (like if they even noticed it)

@paula-stacho paula-stacho merged commit e529bce into main Aug 7, 2025
56 of 58 checks passed
@paula-stacho paula-stacho deleted the COMPASS-9332 branch August 7, 2025 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat feature flagged PRs labeled with this label will not be included in the release notes of the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants