-
Notifications
You must be signed in to change notification settings - Fork 902
ATLAS-5376: Atlas UI: Relationship cards layout breaking, overlapping, and tooltip placement issues with long entity names (React & Classic UI) #737
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,28 +48,40 @@ import { CloseIcon, LightTooltip } from "@components/muiComponents"; | |
| import { useAppSelector } from "@hooks/reducerHook"; | ||
| import { Link as MUILink } from "@mui/material"; | ||
|
|
||
| interface CustomLinkProps { | ||
| href: string; | ||
| status: string; | ||
| guid: string; | ||
| name: string; | ||
| typeName: string; | ||
| params: URLSearchParams | string; | ||
| } | ||
|
|
||
| const CustomLink = ({ | ||
| href, | ||
| status, | ||
| entityColor, | ||
| guid, | ||
| name, | ||
| typeName, | ||
| params | ||
| }: any): any => { | ||
| }: CustomLinkProps): JSX.Element => { | ||
| const displayLabel = typeName ? `${name} (${typeName})` : name; | ||
| return ( | ||
| <li className={status}> | ||
| <MUILink | ||
| component={RouterLink} | ||
| to={{ | ||
| pathname: href, | ||
| search: params.toString() ? params.toString() : "" | ||
| }} | ||
| style={{ color: entityColor }} | ||
| replace={true} | ||
| > | ||
| {name} ({typeName}) | ||
| </MUILink> | ||
| <LightTooltip title={displayLabel}> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (~lines 71–84, 541–549) MUI Tooltip expects a child that holds a ref. MUILink + RouterLink can trigger console warnings. Wrapping in <span style={{ display: 'block', overflow: 'hidden' }}> is safer. |
||
| <MUILink | ||
| component={RouterLink} | ||
| to={{ | ||
| pathname: href, | ||
| search: params.toString() ? params.toString() : "" | ||
| }} | ||
| className={`relationship-node-link ${status.includes("deleted-relation") ? "text-deleted" : "text-active"}`} | ||
| replace={true} | ||
| underline="hover" | ||
| > | ||
| {displayLabel} | ||
| </MUILink> | ||
| </LightTooltip> | ||
| </li> | ||
| ); | ||
| }; | ||
|
|
@@ -79,8 +91,8 @@ const RelationshipLineage = ({ | |
| relationshipAttributes, | ||
| isLoading | ||
| }: { | ||
| entity: Record<string, any>; | ||
| relationshipAttributes?: Record<string, any[]>; | ||
| entity: Record<string, unknown>; | ||
| relationshipAttributes?: Record<string, unknown[]>; | ||
| isLoading?: boolean; | ||
| }) => { | ||
| const entityData = cloneDeep(entity); | ||
|
|
@@ -95,13 +107,13 @@ const RelationshipLineage = ({ | |
| const zoomOutButtonRef = useRef(null); | ||
| const relationshipSVG = useRef(null); | ||
| const [drawerOpen, setDrawerOpen] = useState(false); | ||
| const [nodeDetails, setNodeDetails] = useState<any>({}); | ||
| const [nodeDetails, setNodeDetails] = useState<Record<string, unknown>>({}); | ||
| const [searchTerm, setSearchTerm] = useState(""); | ||
| const [zoomId, setZoomId] = useState(""); | ||
|
|
||
| const createData = (entityData: Record<string, any>) => { | ||
| const createData = (entityData: Record<string, unknown>) => { | ||
| let links = []; | ||
| let nodes: Record<string, any> = {}; | ||
| let nodes: Record<string, unknown> = {}; | ||
| if (entityData && entityData.relationshipAttributes) { | ||
| for (const obj in entityData.relationshipAttributes) { | ||
| if (!isEmpty(entityData.relationshipAttributes[obj])) { | ||
|
|
@@ -146,9 +158,9 @@ const RelationshipLineage = ({ | |
| selectedNodeColor = "#4a90e2"; | ||
|
|
||
| var svg = d3 | ||
| .select(svgElement) | ||
| .attr("viewBox", `${-padding} ${-padding} ${width + padding * 2} ${height + padding * 2}`) | ||
| .attr("enable-background", `new ${-padding} ${-padding} ${width + padding * 2} ${height + padding * 2}`), | ||
| .select(svgElement) | ||
| .attr("viewBox", `${-padding} ${-padding} ${width + padding * 2} ${height + padding * 2}`) | ||
| .attr("enable-background", `new ${-padding} ${-padding} ${width + padding * 2} ${height + padding * 2}`), | ||
| node, | ||
| path; | ||
|
|
||
|
|
@@ -187,7 +199,7 @@ const RelationshipLineage = ({ | |
|
|
||
| var forceLink = d3 | ||
| .forceLink() | ||
| .id(function (d: any) { | ||
| .id(function (d: Record<string, unknown>) { | ||
| return d.id; | ||
| }) | ||
| .distance(function (d) { | ||
|
|
@@ -252,7 +264,7 @@ const RelationshipLineage = ({ | |
| d.radius = 25; | ||
| return d.radius; | ||
| }) | ||
| .attr("fill", function (d: any) { | ||
| .attr("fill", function (d: Record<string, unknown>) { | ||
| if (d && d.value && d.value.guid == guid) { | ||
| if (isAllEntityRelationDeleted({ data: d, type: "node" })) { | ||
| return deletedEntityColor; | ||
|
|
@@ -472,7 +484,6 @@ const RelationshipLineage = ({ | |
| ? " deleted-relation" | ||
| : ""; | ||
| let nodeGuid = options.guid; | ||
| let entityColor = obj.color; | ||
| let name = obj.name; | ||
| let typeName = options.typeName; | ||
| let keys = Array.from(searchParams.keys()); | ||
|
|
@@ -491,7 +502,6 @@ const RelationshipLineage = ({ | |
| <CustomLink | ||
| href={tempLink} | ||
| status={status} | ||
| entityColor={entityColor} | ||
| guid={nodeGuid} | ||
| name={name} | ||
| typeName={typeName} | ||
|
|
@@ -504,7 +514,6 @@ const RelationshipLineage = ({ | |
| <CustomLink | ||
| href={`/detailPage/${nodeGuid}`} | ||
| status={status} | ||
| entityColor={entityColor} | ||
| guid={nodeGuid} | ||
| name={name} | ||
| typeName={typeName} | ||
|
|
@@ -519,23 +528,25 @@ const RelationshipLineage = ({ | |
| let status = entityStateReadOnly[entityStatus] | ||
| ? " deleted-relation" | ||
| : ""; | ||
| let entityColor = obj.color; | ||
| let name = obj.name; | ||
|
|
||
| if (obj.relationship) { | ||
| status = entityStateReadOnly[relationshipStatus] | ||
| ? "deleted-relation" | ||
| : ""; | ||
| } | ||
| const displayLabel = options.typeName ? `${name} (${options.typeName})` : name; | ||
| return ( | ||
| <li className={status}> | ||
| <MUILink | ||
| component={RouterLink} | ||
| to={`/detailPage/${options.guid}?tabActive=relationship`} | ||
| style={{ color: entityColor }} | ||
| > | ||
| {name} ({options.typeName}) | ||
| </MUILink> | ||
| <LightTooltip title={displayLabel}> | ||
| <MUILink | ||
| component={RouterLink} | ||
| to={`/detailPage/${options.guid}?tabActive=relationship`} | ||
| className={`relationship-node-link ${status.includes("deleted-relation") ? "text-deleted" : "text-active"}`} | ||
| > | ||
| {displayLabel} | ||
| </MUILink> | ||
| </LightTooltip> | ||
| </li> | ||
| ); | ||
| }; | ||
|
|
@@ -617,7 +628,7 @@ const RelationshipLineage = ({ | |
| listString.push(<Fragment key={itemKey}>{getElement(data)}</Fragment>); | ||
| } | ||
| return ( | ||
| <Stack sx={{ background: "white" }} minHeight={"150px"} maxWidth="520px"> | ||
| <Stack sx={{ background: "white" }} minHeight={"150px"} maxWidth="350px"> | ||
| {/* {listString?.length > 1 && ( */} | ||
| <Paper | ||
| variant="outlined" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1217,6 +1217,41 @@ describe('RelationshipLineage', () => { | |
| }); | ||
| }); | ||
|
|
||
| describe('Tooltip Rendering', () => { | ||
| it('should render LightTooltip for relationship nodes in drawer', async () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add negative/edge cases: deleted status styling, missing typeName label, and at least one assertion that tooltip title matches full untruncated text. Classic UI tooltip lifecycle has zero automated coverage. |
||
| render( | ||
| <TestWrapper> | ||
| <RelationshipLineage entity={mockEntityWithRelationships} /> | ||
| </TestWrapper> | ||
| ); | ||
|
|
||
| const mockNode = { | ||
| name: 'Process', | ||
| value: [ | ||
| { | ||
| guid: 'proc-1', | ||
| typeName: 'Process', | ||
| displayText: 'Test Tooltip Name', | ||
| entityStatus: 'ACTIVE', | ||
| relationshipStatus: 'ACTIVE' | ||
| } | ||
| ] | ||
| }; | ||
|
|
||
| act(() => { | ||
| if (mockEnterSelection.clickHandler) { | ||
| mockEnterSelection.clickHandler(mockNode); | ||
| } | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| const tooltips = screen.getAllByTestId('light-tooltip'); | ||
| const targetTooltip = tooltips.find(t => t.getAttribute('title') === 'Test Tooltip Name (Process)'); | ||
| expect(targetTooltip).toBeInTheDocument(); | ||
| }, { timeout: 3000 }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('Edge Cases', () => { | ||
| it('should handle entity without relationshipAttributes', () => { | ||
| const entityWithoutAttributes = { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -99,6 +99,13 @@ | |
| margin-bottom: 5px; | ||
| text-align: left; | ||
|
|
||
| &.entity-list-item { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ellipsis is applied to ul > li.entity-list-item, but buildListItem() in RelationshipLayoutView.js (~lines 359–361) puts long text inside . Ellipsis on Consider moving truncation to .entity-list-item .entity-type-name (with display: inline-block; max-width: 100%) — same pattern used for .relationship-card-link in relationship.scss. Truncation on confirm this, and update |
||
| white-space: nowrap; | ||
| overflow: hidden; | ||
| text-overflow: ellipsis; | ||
| max-width: 100%; | ||
| } | ||
|
|
||
| &.deleted-relation { | ||
| .deleteBtn { | ||
| padding: 2px 8px !important; | ||
|
|
||
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.
guid is declared in the interface and destructured but never used.
Remove unused guid from CustomLinkProps and destructuring, or use it (e.g. as key / data-testid).
verify and update