-
Notifications
You must be signed in to change notification settings - Fork 246
feat: improve relationship selected COMPASS-9478 #7135
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
a432c4b to
87b737b
Compare
281416b to
88ea842
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 improves the relationship editing functionality in the data modeling diagram by adding a name field, implementing telemetry tracking, and highlighting relationship fields in the UI to match design specifications.
- Adds telemetry events for relationship operations (add, edit, delete) with relationship count tracking
- Updates the relationship drawer UI with new name field and improved layout using accordions
- Implements field highlighting to visually indicate relationship connections in the diagram
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/compass-telemetry/src/telemetry-events.ts |
Adds three new telemetry event types for relationship operations |
packages/compass-data-modeling/src/store/diagram.ts |
Adds telemetry tracking to relationship actions and helper function for counting relationships |
packages/compass-data-modeling/src/services/data-model-storage.ts |
Adds optional name field to relationship schema |
packages/compass-data-modeling/src/components/relationship-drawer-content.tsx |
Major UI redesign with accordion layout, name field, and improved styling |
packages/compass-data-modeling/src/components/diagram-editor.tsx |
Implements field highlighting for selected relationships |
packages/compass-components/src/components/accordion.tsx |
Adds defaultOpen and textClassName props to Accordion component |
| Test files | Updates test fixtures and test cases to accommodate new features |
Comments suppressed due to low confidence (1)
packages/compass-telemetry/src/telemetry-events.ts:2922
- The event name 'Data Modeling Relationship Form Opened' doesn't match the type name 'DataModelingDiagramRelationshipEdited'. Consider renaming the event to 'Data Modeling Relationship Edited' for consistency.
name: 'Data Modeling Relationship Form Opened';
packages/compass-data-modeling/src/components/relationship-drawer-content.tsx
Show resolved
Hide resolved
|
@bsradcliffe for an early review. disclaimer - the drawer is still a placeholder, focus is on the content (form) and highlighting the relationship in the diagram. |
gribnoysup
left a comment
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 comment I would like to hear your opinion on, otherwise looks very good!
|
|
||
| expect(screen.getByTestId('my-test-id')).to.exist; | ||
| const button = screen.getByText('Accordion Test'); | ||
| fireEvent.click(button); |
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.
Thanks for cleaning-up!
packages/compass-data-modeling/src/components/diagram-editor.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Sergey Petushkov <[email protected]>
Description
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes