Skip to content

Commit aa663ea

Browse files
dlasecki-boxdlasecki-box
committed
feat(bounding-box): make checks more strict
1 parent 2eae467 commit aa663ea

File tree

4 files changed

+48
-42
lines changed

4 files changed

+48
-42
lines changed

‎src/elements/content-sidebar/MetadataSidebarRedesign.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ interface PropsWithoutContext extends ExternalProps {
6161
fileExtension?: string;
6262
fileId: string;
6363
filteredTemplateIds?: string[];
64-
getPreview?: () => {
64+
getPreview: () => {
6565
showBoundingBoxHighlights?: (boundingBoxes: BoxAnnotationsBoundingBox[]) => void;
6666
hideBoundingBoxHighlights?: () => void;
6767
};

‎src/elements/content-sidebar/__tests__/useMetadataFieldSelection.test.ts‎

Lines changed: 22 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,15 @@ import { type MetadataTemplateField } from '@box/metadata-editor';
22
import { act, renderHook } from '../../../test-utils/testing-library';
33
import useMetadataFieldSelection from '../hooks/useMetadataFieldSelection';
44

5+
const mockTargetLocation = [
6+
{
7+
itemId: 'item-1',
8+
page: 0,
9+
text: 'test text',
10+
boundingBox: { left: 0.1, top: 0.2, right: 0.5, bottom: 0.6 },
11+
},
12+
];
13+
514
const createMockField = (overrides: Partial<MetadataTemplateField> = {}): MetadataTemplateField => ({
615
id: 'field-1',
716
key: 'testField',
@@ -34,7 +43,7 @@ describe('useMetadataFieldSelection', () => {
3443
const { result } = renderHook(() => useMetadataFieldSelection(getPreview));
3544

3645
act(() => {
37-
result.current.handleSelectMetadataField(createMockField());
46+
result.current.handleSelectMetadataField(createMockField({ targetLocation: mockTargetLocation }));
3847
});
3948

4049
expect(result.current.selectedMetadataFieldId).toBe('field-1');
@@ -70,21 +79,22 @@ describe('useMetadataFieldSelection', () => {
7079
]);
7180
});
7281

73-
test('should not call showBoundingBoxHighlights when field has no targetLocation', () => {
82+
test('should not select field or show highlights when field has no targetLocation', () => {
7483
const { result } = renderHook(() => useMetadataFieldSelection(getPreview));
7584

7685
act(() => {
7786
result.current.handleSelectMetadataField(createMockField());
7887
});
7988

89+
expect(result.current.selectedMetadataFieldId).toBeNull();
8090
expect(mockShowBoundingBoxHighlights).not.toHaveBeenCalled();
8191
});
8292

8393
test('should deselect field and hide highlights when handleSelectMetadataField is called with null', () => {
8494
const { result } = renderHook(() => useMetadataFieldSelection(getPreview));
8595

8696
act(() => {
87-
result.current.handleSelectMetadataField(createMockField());
97+
result.current.handleSelectMetadataField(createMockField({ targetLocation: mockTargetLocation }));
8898
});
8999

90100
expect(result.current.selectedMetadataFieldId).toBe('field-1');
@@ -101,7 +111,7 @@ describe('useMetadataFieldSelection', () => {
101111
const { result } = renderHook(() => useMetadataFieldSelection(getPreview));
102112

103113
act(() => {
104-
result.current.handleSelectMetadataField(createMockField());
114+
result.current.handleSelectMetadataField(createMockField({ targetLocation: mockTargetLocation }));
105115
});
106116

107117
act(() => {
@@ -114,44 +124,24 @@ describe('useMetadataFieldSelection', () => {
114124
expect(mockHideBoundingBoxHighlights).toHaveBeenCalled();
115125
});
116126

117-
test('should not crash when getPreview is undefined', () => {
118-
const { result } = renderHook(() => useMetadataFieldSelection(undefined));
119-
120-
act(() => {
121-
result.current.handleSelectMetadataField(
122-
createMockField({
123-
targetLocation: [
124-
{
125-
itemId: 'item-1',
126-
page: 0,
127-
text: 'test text',
128-
boundingBox: { left: 0.1, top: 0.2, right: 0.5, bottom: 0.6 },
129-
},
130-
],
131-
}),
132-
);
133-
});
134-
135-
expect(result.current.selectedMetadataFieldId).toBe('field-1');
136-
});
137-
138-
test('should not crash when getPreview returns undefined', () => {
127+
test('should not select field or crash when getPreview returns undefined', () => {
139128
const getPreviewReturningUndefined = jest.fn().mockReturnValue(undefined);
140129
const { result } = renderHook(() => useMetadataFieldSelection(getPreviewReturningUndefined));
141130

142131
act(() => {
143-
result.current.handleSelectMetadataField(createMockField());
132+
result.current.handleSelectMetadataField(createMockField({ targetLocation: mockTargetLocation }));
144133
});
145134

146-
expect(result.current.selectedMetadataFieldId).toBe('field-1');
135+
expect(result.current.selectedMetadataFieldId).toBeNull();
136+
expect(mockShowBoundingBoxHighlights).not.toHaveBeenCalled();
147137
});
148138

149139
describe('click outside behavior', () => {
150140
test('should deselect when clicking outside metadata field and bounding box elements', () => {
151141
const { result } = renderHook(() => useMetadataFieldSelection(getPreview));
152142

153143
act(() => {
154-
result.current.handleSelectMetadataField(createMockField());
144+
result.current.handleSelectMetadataField(createMockField({ targetLocation: mockTargetLocation }));
155145
});
156146

157147
expect(result.current.selectedMetadataFieldId).toBe('field-1');
@@ -169,7 +159,7 @@ describe('useMetadataFieldSelection', () => {
169159
const { result } = renderHook(() => useMetadataFieldSelection(getPreview));
170160

171161
act(() => {
172-
result.current.handleSelectMetadataField(createMockField());
162+
result.current.handleSelectMetadataField(createMockField({ targetLocation: mockTargetLocation }));
173163
});
174164

175165
const metadataFieldElement = document.createElement('div');
@@ -190,7 +180,7 @@ describe('useMetadataFieldSelection', () => {
190180
const { result } = renderHook(() => useMetadataFieldSelection(getPreview));
191181

192182
act(() => {
193-
result.current.handleSelectMetadataField(createMockField());
183+
result.current.handleSelectMetadataField(createMockField({ targetLocation: mockTargetLocation }));
194184
});
195185

196186
const boundingBoxElement = document.createElement('div');
@@ -222,7 +212,7 @@ describe('useMetadataFieldSelection', () => {
222212
const { result, unmount } = renderHook(() => useMetadataFieldSelection(getPreview));
223213

224214
act(() => {
225-
result.current.handleSelectMetadataField(createMockField());
215+
result.current.handleSelectMetadataField(createMockField({ targetLocation: mockTargetLocation }));
226216
});
227217

228218
unmount();

‎src/elements/content-sidebar/hooks/useMetadataFieldSelection.ts‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const METADATA_FIELD_SELECTOR = '[data-metadata-field]';
2727
const BOUNDING_BOX_SELECTOR = '.ba-BoundingBoxHighlightRect';
2828

2929
function useMetadataFieldSelection(
30-
getPreview?: () => {
30+
getPreview: () => {
3131
showBoundingBoxHighlights?: (boundingBoxes: BoxAnnotationsBoundingBox[]) => void;
3232
hideBoundingBoxHighlights?: () => void;
3333
},
@@ -36,7 +36,7 @@ function useMetadataFieldSelection(
3636

3737
const handleDeselectMetadataField = useCallback(() => {
3838
setSelectedMetadataFieldId(null);
39-
const preview = getPreview?.();
39+
const preview = getPreview();
4040
if (!preview) {
4141
return;
4242
}
@@ -73,17 +73,15 @@ function useMetadataFieldSelection(
7373
return;
7474
}
7575

76-
setSelectedMetadataFieldId(field.id);
76+
const preview = getPreview();
77+
const boundingBoxes = convertTargetLocationToBoundingBox(field.id, field.targetLocation);
7778

78-
const preview = getPreview?.();
79-
if (!preview) {
79+
if (!preview || !boundingBoxes || !preview.showBoundingBoxHighlights) {
8080
return;
8181
}
8282

83-
const boundingBoxes = convertTargetLocationToBoundingBox(field.id, field.targetLocation);
84-
if (boundingBoxes) {
85-
preview.showBoundingBoxHighlights?.(boundingBoxes);
86-
}
83+
setSelectedMetadataFieldId(field.id);
84+
preview.showBoundingBoxHighlights(boundingBoxes);
8785
},
8886
[getPreview, handleDeselectMetadataField],
8987
);

‎src/elements/content-sidebar/types/BoxAISidebarTypes.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,28 @@ export type BoxAISidebarCacheSetter = (
1515
) => void;
1616

1717
export type BoxAnnotationsBoundingBox = {
18+
/**
19+
* The unique id for the bounding box (used for UI state only)
20+
*/
1821
id: string;
22+
/**
23+
* The x coordinate of the bounding box starting point (0-100 percentage of the width of the page)
24+
*/
1925
x: number;
26+
/**
27+
* The y coordinate of the bounding box starting point (0-100 percentage of the height of the page)
28+
*/
2029
y: number;
30+
/**
31+
* The width of the bounding box (0-100 percentage of the width of the page)
32+
*/
2133
width: number;
34+
/**
35+
* The height of the bounding box (0-100 percentage of the height of the page)
36+
*/
2237
height: number;
38+
/**
39+
* The page number of the bounding box (1-indexed)
40+
*/
2341
pageNumber: number;
2442
};

0 commit comments

Comments
 (0)