Skip to content

Commit 16fa8a8

Browse files
AnemyAnemy
committed
fixup: review comments, cleanup, add tests
1 parent e7c64bf commit 16fa8a8

File tree

12 files changed

+119
-82
lines changed

12 files changed

+119
-82
lines changed

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
"@testing-library/jest-dom": "^6.6.3",
7575
"@testing-library/react": "^12.1.5",
7676
"@testing-library/react-hooks": "^8.0.1",
77+
"@testing-library/user-event": "^14.6.1",
7778
"@types/d3-path": "^3",
7879
"@types/jest": "30.0.0",
7980
"@types/node": "22.15.29",

‎src/components/field/field-type-content.tsx‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { useMemo } from 'react';
2+
3+
import { ObjectFieldType } from '@/components/field/object-field-type';
4+
import { useEditableDiagramInteractions } from '@/hooks/use-editable-diagram-interactions';
5+
6+
export const FieldTypeContent = ({
7+
type,
8+
nodeId,
9+
id,
10+
}: {
11+
id: string | string[];
12+
nodeId: string;
13+
type: React.ReactNode;
14+
}) => {
15+
const { onClickAddFieldToObjectField: _onClickAddFieldToObjectField } = useEditableDiagramInteractions();
16+
17+
const onClickAddFieldToObject = useMemo(
18+
() =>
19+
_onClickAddFieldToObjectField
20+
? (event: React.MouseEvent<HTMLButtonElement>) => {
21+
event.stopPropagation();
22+
_onClickAddFieldToObjectField(event, nodeId, Array.isArray(id) ? id : [id]);
23+
}
24+
: undefined,
25+
[_onClickAddFieldToObjectField, nodeId, id],
26+
);
27+
28+
if (type === 'object' && !!onClickAddFieldToObject) {
29+
return (
30+
<ObjectFieldType
31+
data-testid={`object-field-type-${nodeId}-${typeof id === 'string' ? id : id.join('.')}`}
32+
onClickAddFieldToObject={onClickAddFieldToObject}
33+
/>
34+
);
35+
}
36+
37+
return <>{type}</>;
38+
};

‎src/components/field/field.test.tsx‎

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { palette } from '@leafygreen-ui/palette';
22
import { ComponentProps } from 'react';
3+
import { userEvent } from '@testing-library/user-event';
34

45
import { render, screen } from '@/mocks/testing-utils';
56
import { Field as FieldComponent } from '@/components/field/field';
@@ -12,20 +13,19 @@ const Field = (props: React.ComponentProps<typeof FieldComponent>) => (
1213
</EditableDiagramInteractionsProvider>
1314
);
1415

15-
const noop = () => {
16-
/* no operation */
16+
const FieldWithEditableInteractions = ({
17+
onAddFieldToObjectFieldClick,
18+
...fieldProps
19+
}: React.ComponentProps<typeof FieldComponent> & {
20+
onAddFieldToObjectFieldClick?: () => void;
21+
}) => {
22+
return (
23+
<EditableDiagramInteractionsProvider onAddFieldToObjectFieldClick={onAddFieldToObjectFieldClick}>
24+
<FieldComponent {...fieldProps} />
25+
</EditableDiagramInteractionsProvider>
26+
);
1727
};
1828

19-
const FieldWithEditableInteractions = (props: React.ComponentProps<typeof FieldComponent>) => (
20-
<EditableDiagramInteractionsProvider
21-
onFieldClick={noop}
22-
onAddFieldToNodeClick={noop}
23-
onAddFieldToObjectFieldClick={noop}
24-
>
25-
<FieldComponent {...props} />
26-
</EditableDiagramInteractionsProvider>
27-
);
28-
2929
describe('field', () => {
3030
const DEFAULT_PROPS: ComponentProps<typeof Field> = {
3131
nodeType: 'collection',
@@ -52,14 +52,34 @@ describe('field', () => {
5252
expect(button).not.toBeInTheDocument();
5353
});
5454
describe('With editable interactions supplied', () => {
55-
it('Should have a button to add a field on an object type', () => {
56-
render(<FieldWithEditableInteractions {...DEFAULT_PROPS} type={'object'} id={['ordersId']} />);
55+
it('Should have a button to add a field on an object type', async () => {
56+
const onAddFieldToObjectFieldClickMock = vi.fn();
57+
58+
render(
59+
<FieldWithEditableInteractions
60+
{...DEFAULT_PROPS}
61+
type={'object'}
62+
id={['ordersId']}
63+
onAddFieldToObjectFieldClick={onAddFieldToObjectFieldClickMock}
64+
/>,
65+
);
5766
expect(screen.getByText('ordersId')).toBeInTheDocument();
5867
expect(screen.getByText('{}')).toBeInTheDocument();
5968
const button = screen.getByRole('button');
6069
expect(button).toBeInTheDocument();
6170
expect(button).toHaveAttribute('data-testid', 'object-field-type-pineapple-ordersId');
6271
expect(button).toHaveAttribute('title', 'Add Field');
72+
expect(onAddFieldToObjectFieldClickMock).not.toHaveBeenCalled();
73+
await userEvent.click(button);
74+
expect(onAddFieldToObjectFieldClickMock).toHaveBeenCalled();
75+
});
76+
77+
it('Should not have a button to add a field with non-object types', () => {
78+
render(<FieldWithEditableInteractions {...DEFAULT_PROPS} id={['ordersId']} />);
79+
expect(screen.getByText('ordersId')).toBeInTheDocument();
80+
expect(screen.getByText('objectId')).toBeInTheDocument();
81+
const button = screen.queryByRole('button');
82+
expect(button).not.toBeInTheDocument();
6383
});
6484
});
6585
describe('With glyphs', () => {

‎src/components/field/field.tsx‎

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@ import { palette } from '@leafygreen-ui/palette';
44
import Icon from '@leafygreen-ui/icon';
55
import { useDarkMode } from '@leafygreen-ui/leafygreen-provider';
66
import { useTheme } from '@emotion/react';
7-
import { MouseEvent as ReactMouseEvent, useMemo } from 'react';
7+
import { useMemo } from 'react';
88

99
import { animatedBlueBorder, ellipsisTruncation } from '@/styles/styles';
1010
import { DEFAULT_DEPTH_SPACING, DEFAULT_FIELD_HEIGHT } from '@/utilities/constants';
1111
import { FieldDepth } from '@/components/field/field-depth';
12-
import { ObjectFieldType } from '@/components/field/object-field-type';
12+
import { FieldTypeContent } from '@/components/field/field-type-content';
1313
import { NodeField, NodeGlyph, NodeType } from '@/types';
1414
import { PreviewGroupArea } from '@/utilities/get-preview-group-area';
1515
import { useEditableDiagramInteractions } from '@/hooks/use-editable-diagram-interactions';
@@ -133,38 +133,6 @@ interface Props extends NodeField {
133133
selectedGroupHeight?: number;
134134
}
135135

136-
function FieldTypeContent({
137-
type,
138-
nodeId,
139-
id,
140-
}: {
141-
id: string | string[];
142-
} & Pick<Props, 'type' | 'nodeId'>) {
143-
const { onClickAddFieldToObjectField: _onClickAddFieldToObjectField } = useEditableDiagramInteractions();
144-
145-
const onClickAddFieldToObject = useMemo(
146-
() =>
147-
_onClickAddFieldToObjectField && Array.isArray(id)
148-
? (event: React.MouseEvent<HTMLButtonElement>) => {
149-
event.stopPropagation();
150-
_onClickAddFieldToObjectField(event, nodeId, id);
151-
}
152-
: undefined,
153-
[_onClickAddFieldToObjectField, nodeId, id],
154-
);
155-
156-
if (type === 'object' && !!onClickAddFieldToObject) {
157-
return (
158-
<ObjectFieldType
159-
data-testid={`object-field-type-${nodeId}-${typeof id === 'string' ? id : id.join('.')}`}
160-
onClickAddFieldToObject={onClickAddFieldToObject}
161-
/>
162-
);
163-
}
164-
165-
return <>{type}</>;
166-
}
167-
168136
export const Field = ({
169137
hoverVariant,
170138
isHovering = false,
@@ -203,7 +171,7 @@ export const Field = ({
203171
? {
204172
'data-testid': `selectable-field-${nodeId}-${typeof id === 'string' ? id : id.join('.')}`,
205173
selectable: true,
206-
onClick: (event: ReactMouseEvent) => onClickField(event, { id, nodeId }),
174+
onClick: (event: React.MouseEvent) => onClickField(event, { id, nodeId }),
207175
}
208176
: undefined;
209177
}, [onClickField, selectable, id, nodeId]);

‎src/components/field/object-field-type.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ const AddNestedFieldIconButton = styled.button`
5050
&[data-hover='true'],
5151
&:focus-visible,
5252
&[data-focus='true'] {
53-
color: ${props => props.theme.node.fieldIconButtonHover};
53+
color: ${palette.black};
5454
5555
&::before {
5656
background-color: ${props => props.theme.node.fieldIconButtonHoverBackground};

‎src/components/icons/plus-with-square.tsx‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,13 @@
1-
import { useMemo } from 'react';
21
import { useTheme } from '@emotion/react';
32

43
export const PlusWithSquare: React.FunctionComponent = () => {
54
const theme = useTheme();
65

7-
const strokeColor = useMemo(() => theme.node.headerIcon, [theme]);
8-
96
return (
107
<svg width="14" height="14" viewBox="0 0 14 14" fill="none" xmlns="http://www.w3.org/2000/svg">
118
<path
129
d="M12 0.75H2C1.66848 0.75 1.35054 0.881696 1.11612 1.11612C0.881696 1.35054 0.75 1.66848 0.75 2V12C0.75 12.3315 0.881696 12.6495 1.11612 12.8839C1.35054 13.1183 1.66848 13.25 2 13.25H12C12.3315 13.25 12.6495 13.1183 12.8839 12.8839C13.1183 12.6495 13.25 12.3315 13.25 12V2C13.25 1.66848 13.1183 1.35054 12.8839 1.11612C12.6495 0.881696 12.3315 0.75 12 0.75ZM11.75 11.75H2.25V2.25H11.75V11.75ZM3.75 7C3.75 6.80109 3.82902 6.61032 3.96967 6.46967C4.11032 6.32902 4.30109 6.25 4.5 6.25H6.25V4.5C6.25 4.30109 6.32902 4.11032 6.46967 3.96967C6.61032 3.82902 6.80109 3.75 7 3.75C7.19891 3.75 7.38968 3.82902 7.53033 3.96967C7.67098 4.11032 7.75 4.30109 7.75 4.5V6.25H9.5C9.69891 6.25 9.88968 6.32902 10.0303 6.46967C10.171 6.61032 10.25 6.80109 10.25 7C10.25 7.19891 10.171 7.38968 10.0303 7.53033C9.88968 7.67098 9.69891 7.75 9.5 7.75H7.75V9.5C7.75 9.69891 7.67098 9.88968 7.53033 10.0303C7.38968 10.171 7.19891 10.25 7 10.25C6.80109 10.25 6.61032 10.171 6.46967 10.0303C6.32902 9.88968 6.25 9.69891 6.25 9.5V7.75H4.5C4.30109 7.75 4.11032 7.67098 3.96967 7.53033C3.82902 7.38968 3.75 7.19891 3.75 7Z"
13-
fill={strokeColor}
10+
fill={theme.node.headerIcon}
1411
/>
1512
</svg>
1613
);

‎src/components/node/node.test.tsx‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
import { screen } from '@testing-library/react';
22
import { NodeProps, useViewport } from '@xyflow/react';
3+
import { userEvent } from '@testing-library/user-event';
34

45
import { render } from '@/mocks/testing-utils';
56
import { InternalNode } from '@/types/internal';
67
import { Node as NodeComponent } from '@/components/node/node';
78
import { EditableDiagramInteractionsProvider } from '@/hooks/use-editable-diagram-interactions';
89

9-
const Node = (props: React.ComponentProps<typeof NodeComponent>) => (
10-
<EditableDiagramInteractionsProvider>
10+
const Node = ({
11+
onAddFieldToNodeClick,
12+
...props
13+
}: React.ComponentProps<typeof NodeComponent> & {
14+
onAddFieldToNodeClick?: () => void;
15+
}) => (
16+
<EditableDiagramInteractionsProvider onAddFieldToNodeClick={onAddFieldToNodeClick}>
1117
<NodeComponent {...props} />
1218
</EditableDiagramInteractionsProvider>
1319
);
@@ -67,6 +73,8 @@ describe('node', () => {
6773
expect(screen.getByText('employees')).toBeInTheDocument();
6874
expect(screen.getByText('employeeId')).toBeInTheDocument();
6975
expect(screen.getByText('string')).toBeInTheDocument();
76+
const button = screen.queryByRole('button', { name: 'Add Field' });
77+
expect(button).not.toBeInTheDocument();
7078
});
7179

7280
it('Should show contextual zoom', () => {
@@ -84,4 +92,16 @@ describe('node', () => {
8492
expect(screen.queryByText('employeeId')).not.toBeVisible();
8593
expect(screen.queryByText('string')).not.toBeVisible();
8694
});
95+
96+
it('Should show a clickable button to add a field when add field is supplied', async () => {
97+
const onAddFieldToNodeClickMock = vi.fn();
98+
99+
render(<Node {...DEFAULT_PROPS} onAddFieldToNodeClick={onAddFieldToNodeClickMock} />);
100+
const button = screen.getByRole('button', { name: 'Add Field' });
101+
expect(button).toBeInTheDocument();
102+
expect(button).toHaveAttribute('title', 'Add Field');
103+
expect(onAddFieldToNodeClickMock).not.toHaveBeenCalled();
104+
await userEvent.click(button);
105+
expect(onAddFieldToNodeClickMock).toHaveBeenCalled();
106+
});
87107
});

‎src/components/node/node.tsx‎

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { fontFamilies, spacing } from '@leafygreen-ui/tokens';
44
import { useTheme } from '@emotion/react';
55
import Icon from '@leafygreen-ui/icon';
66
import IconButton from '@leafygreen-ui/icon-button';
7-
import { useMemo, useState } from 'react';
7+
import { useCallback, useState } from 'react';
88

99
import { DEFAULT_NODE_HEADER_HEIGHT, ZOOM_THRESHOLD } from '@/utilities/constants';
1010
import { InternalNode } from '@/types/internal';
@@ -119,26 +119,15 @@ export const Node = ({
119119

120120
const [isHovering, setHovering] = useState(false);
121121

122-
const { onClickAddFieldToNode } = useEditableDiagramInteractions();
122+
const { onClickAddFieldToNode: addFieldToNodeClickHandler } = useEditableDiagramInteractions();
123123

124-
const actions = useMemo(() => {
125-
if (!onClickAddFieldToNode) {
126-
return;
127-
}
128-
129-
return (
130-
<AddNewFieldIconButtonButton
131-
aria-label="Add Field"
132-
onClick={(event: React.MouseEvent<HTMLButtonElement>) => {
133-
event.stopPropagation();
134-
onClickAddFieldToNode(event, id);
135-
}}
136-
title="Add Field"
137-
>
138-
<PlusWithSquare />
139-
</AddNewFieldIconButtonButton>
140-
);
141-
}, [onClickAddFieldToNode, id]);
124+
const onClickAddFieldToNode = useCallback(
125+
(event: React.MouseEvent<HTMLButtonElement>) => {
126+
event.stopPropagation();
127+
addFieldToNodeClickHandler?.(event, id);
128+
},
129+
[addFieldToNodeClickHandler, id],
130+
);
142131

143132
const getAccent = () => {
144133
if (disabled && !isHovering) {
@@ -214,7 +203,11 @@ export const Node = ({
214203
<Icon fill={theme.node.headerIcon} glyph="Drag" />
215204
</NodeHeaderIcon>
216205
<NodeHeaderTitle>{title}</NodeHeaderTitle>
217-
{actions}
206+
{addFieldToNodeClickHandler && (
207+
<AddNewFieldIconButtonButton aria-label="Add Field" onClick={onClickAddFieldToNode} title="Add Field">
208+
<PlusWithSquare />
209+
</AddNewFieldIconButtonButton>
210+
)}
218211
</NodeHeader>
219212
<FieldList nodeId={id} nodeType={type as NodeType} isHovering={isHovering} fields={fields} />
220213
</NodeWithFields>

‎src/styles/theme-dark.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Theme } from '@emotion/react';
22
import { palette } from '@leafygreen-ui/palette';
3+
import { color } from '@leafygreen-ui/tokens';
34

45
import { hexToRgb } from '@/styles/utils';
56

@@ -31,8 +32,7 @@ export const DARK_THEME: Theme = {
3132
disabledHeader: palette.gray.dark3,
3233
disabledColor: palette.gray.dark1,
3334
headerIcon: palette.gray.light2,
34-
fieldIconButton: palette.gray.light1,
35-
fieldIconButtonHover: palette.black,
35+
fieldIconButton: color.dark.icon.primary.default,
3636
fieldIconButtonHoverBackground: hexToRgb(palette.gray.light2, 0.1),
3737
},
3838
};

‎src/styles/theme-light.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Theme } from '@emotion/react';
22
import { palette } from '@leafygreen-ui/palette';
3+
import { color } from '@leafygreen-ui/tokens';
34

45
import { hexToRgb } from '@/styles/utils';
56

@@ -29,8 +30,7 @@ export const LIGHT_THEME: Theme = {
2930
disabledHeader: palette.gray.light3,
3031
disabledColor: palette.gray.light1,
3132
headerIcon: palette.gray.dark1,
32-
fieldIconButton: palette.gray.dark1,
33-
fieldIconButtonHover: palette.black,
33+
fieldIconButton: color.light.icon.primary.default,
3434
fieldIconButtonHoverBackground: hexToRgb(palette.gray.dark2, 0.1),
3535
},
3636
};

0 commit comments

Comments
 (0)