-
Notifications
You must be signed in to change notification settings - Fork 228
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
Conversation
active={isInRelationshipDrawingMode} | ||
aria-pressed={isInRelationshipDrawingMode} | ||
> | ||
<Icon glyph="Relationship"></Icon> |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
b371164
to
511887c
Compare
There was a problem hiding this 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 |
333f3a0
to
4c32197
Compare
packages/compass-data-modeling/src/components/diagram-editor-toolbar.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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
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) |
b19cf5e
to
b0e8c89
Compare
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
Motivation and Context
Open Questions
Dependents
Types of changes