Conversation
…ture-stack/lectern into 398-table-view-interactions
| {isForeignKey && ( | ||
| <button | ||
| css={viewInDiagramButtonStyle(theme)} | ||
| onClick={() => openFocusedDiagram({ schemaName: schema!.name, fieldName: currentSchemaField.name })} |
There was a problem hiding this comment.
Using the non-null assertion is usually an indicator that something can be improved. Try to avoid using this going forward. Check if schema exists before making the openFocusedDiagram call.
| {isForeignKey && ( | ||
| <button | ||
| css={viewInDiagramButtonStyle(theme)} | ||
| onClick={() => openFocusedDiagram({ schemaName: schema!.name, fieldName: currentSchemaField.name })} |
| const isUniqueKey = !!(schema && currentSchemaField) && isFieldUniqueKey(schema, currentSchemaField); | ||
| const isForeignKey = !!(schema && currentSchemaField) && isFieldForeignKey(schema, currentSchemaField); |
| onClick={() => | ||
| schema && | ||
| currentSchemaField && | ||
| openFocusedDiagram({ schemaName: schema.name, fieldName: currentSchemaField.name }) | ||
| } |
There was a problem hiding this comment.
This is technically not wrong, but this can get pretty unreadable as more conditions are added.
I suggest using a more explicit approach:
if (!schema || !currentSchemaField) return;
openFocusedDiagram({
schemaName: schema.name,
fieldName: currentSchemaField.name,
});or you can disable the button entirely.
<button
css={diagramLinkStyle(theme)}
disabled={schema && currentSchemaField}
onClick={() => openFocusedDiagram({ schemaName: schema.name, fieldName: currentSchemaField.name })
}
/>| */ | ||
| export function EntityRelationshipDiagramContent({ dictionary }: EntityRelationshipDiagramProps) { | ||
| const { activeEdgeIds, activateRelationship, deactivateRelationship, relationshipMap } = useActiveRelationship(); | ||
| export function RelationshipDiagramContent({ dictionary, focusField }: RelationshipDiagramContentProps) { |
There was a problem hiding this comment.
This component can be simplified.
fieldKey,fkIndexes,getLayoutedDiagram and RelationshipDiagramFlow are declared multiple times. Lets try not to duplicate declarations here. We can start with declaring our variables at the top:
const fieldKey = focusField ? `${focusField.schemaName}::${focusField.fieldName}` : null;
const fkIndices = fieldKey ? relationshipMap.fieldKeyToFkIndices.get(fieldKey) : null;
let currentNodes = dictionary.schemas;
let currentEdges = allEdges;We can move the if (focusField) { ... } above the useEffect and remove the repeat RelationshipDiagramFlow and getLayoutedDiagram. This conditional will just focus on redefining currentNodes and currentEdges with your filter logic.
Then we can calculate const { nodes, edges } = getLayoutedDiagram(currentNodes, currentEdges);
Adjust useEffect accordingly.
This component should just return the following at the end return <RelationshipDiagramFlow nodes={nodes} edges={edges} isFocused={!!focusField} />; at the end
Related to issue: #398
Summary
Enhances the table view to display visual key indicators (PK/FK) in the Attribute column and enable navigation from FK relationships to the ERD diagram modal with highlighted relationships.
Description of Changes
Readiness Checklist
.env.schemafile and documented in the README