Skip to content

Commit

Permalink
fix(highlight): Ignore empty rects when rendering highlight canvas re…
Browse files Browse the repository at this point in the history
…cts (#697)

* fix(highlight): Ignore empty rects when rendering highlight canvas rects

* fix(highlight): Prevent empty rects from being sent to the backend

* fix(highlight): Update highlight util unit test

* fix(highlight): Update highlight canvas unit test

* fix(highlight): Update highlight canvas unit test per feedback
  • Loading branch information
karelee7 authored Aug 2, 2022
1 parent f725c4c commit fba2523
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 1 deletion.
6 changes: 6 additions & 0 deletions src/highlight/HighlightCanvas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ export default class HighlightCanvas extends React.PureComponent<Props> {

shapesArray.forEach(rect => {
const { height, isActive, isHover, width, x, y } = rect;

// Ignore empty rects with a width or height of 0
if (width === 0 || height === 0) {
return;
}

const rectHeight = (height / 100) * canvasHeight;
const rectWidth = (width / 100) * canvasWidth;
const x1 = (x / 100) * canvasWidth;
Expand Down
23 changes: 23 additions & 0 deletions src/highlight/__mocks__/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,22 @@ export const rect: Rect = {
y: 5,
};

export const noWidthRect: Rect = {
type: 'rect' as const,
height: 10,
width: 0,
x: 5,
y: 5,
};

export const noHeightRect: Rect = {
type: 'rect' as const,
height: 0,
width: 20,
x: 5,
y: 5,
};

export const target: TargetHighlight = {
location: {
type: 'page' as const,
Expand Down Expand Up @@ -55,3 +71,10 @@ export const selection = {
},
],
};

export const canvasContext = {
fillRect: jest.fn(),
restore: jest.fn(),
save: jest.fn(),
strokeRect: jest.fn(),
};
20 changes: 19 additions & 1 deletion src/highlight/__tests__/HighlightCanvas-test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { shallow, ShallowWrapper } from 'enzyme';
import HighlightCanvas, { Props } from '../HighlightCanvas';
import { rect as mockRect } from '../__mocks__/data';
import { canvasContext, rect as mockRect, noWidthRect, noHeightRect } from '../__mocks__/data';

describe('HighlightCanvas', () => {
const defaults: Props = {
Expand Down Expand Up @@ -56,5 +56,23 @@ describe('HighlightCanvas', () => {
const wrapper = getWrapper();
expect(wrapper.find('canvas').hasClass('ba-HighlightCanvas')).toBe(true);
});

test('should render, excluding 0 width and height values', () => {
const wrapper = getWrapper({ shapes: [mockRect, noWidthRect, noHeightRect] });
const instance = wrapper.instance() as HighlightCanvas;

instance.canvasRef = {
current: {
// eslint-disable-next-line @typescript-eslint/ban-ts-ignore
// @ts-ignore
getContext: () => canvasContext,
},
};

instance.renderRects();

expect(canvasContext.fillRect).toHaveBeenCalledTimes(1);
expect(canvasContext.strokeRect).toHaveBeenCalledTimes(1);
});
});
});
40 changes: 40 additions & 0 deletions src/highlight/__tests__/highlightUtil-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,46 @@ describe('highlightUtil', () => {
expect(result[1]).toEqual(rects[3]);
expect(result[2]).toEqual(rects[4]);
});

test('should combine rects by row, excluding 0 width and height values', () => {
const validRect1 = {
height: 20,
width: 100,
x: 100,
y: 200,
};

const validRect2 = {
height: 21,
width: 100,
x: 500,
y: 200,
};

const noHeightRect = {
height: 0,
width: 100,
x: 200,
y: 200,
};

const noWidthRect = {
height: 23,
width: 0,
x: 400,
y: 300,
};

const result = combineRects([validRect1, noHeightRect, validRect2, noWidthRect]);
expect(result).toHaveLength(2);
expect(result[0]).toEqual({
height: 20,
width: 100,
x: 100,
y: 200,
});
expect(result[1]).toEqual(validRect2);
});
});

describe('getShapeRelativeToContainer', () => {
Expand Down
6 changes: 6 additions & 0 deletions src/highlight/highlightUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ export const combineRects = (rects: Shape[]): Shape[] => {

dedupeRects(rects).forEach(rect => {
const { height, width, x, y } = rect;

// Ignore empty rects with a width or height of 0
if (width === 0 || height === 0) {
return;
}

const lastRect = result.pop();

if (!lastRect) {
Expand Down

0 comments on commit fba2523

Please sign in to comment.