Skip to content

Commit

Permalink
fix: LEAP-240: Prevent creating region on clicking outside selected o…
Browse files Browse the repository at this point in the history
…ne (HumanSignal#1667)

* fix: LEAP-240: Prevent creating region on clicking outside selected one

* Add tests for clicking outside selected region

* Ensure that unselecting labels and regions was initiated only by click.

Updated the handleDeferredMouseDown method to accept a boolean argument that represents whether the mouse was clicked or not. This modification prevents unwanted deselection of regions and labels by ensuring that this event occurs only when the mouse was clicked and deselection is intentional.

* Add tests for checking labels behaviour

---------

Co-authored-by: Gondragos <[email protected]>
  • Loading branch information
2 people authored and MasherJames committed Feb 29, 2024
1 parent 85598e1 commit 01a12b4
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 11 deletions.
41 changes: 30 additions & 11 deletions src/components/ImageView/ImageView.js
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,9 @@ export default observer(
crosshairRef = createRef();
handleDeferredMouseDown = null;
deferredClickTimeout = [];
skipMouseUp = false;
skipNextMouseDown = false;
skipNextClick = false;
skipNextMouseUp = false;
mouseDownPoint = null;

constructor(props) {
Expand All @@ -520,10 +522,10 @@ export default observer(
const { item } = this.props;

if (isFF(FF_DEV_1442)) {
this.handleDeferredMouseDown?.();
this.handleDeferredMouseDown?.(true);
}
if (this.skipMouseUp) {
this.skipMouseUp = false;
if (this.skipNextClick) {
this.skipNextClick = false;
return;
}

Expand Down Expand Up @@ -556,8 +558,8 @@ export default observer(
};

handleDeferredClick = (handleDeferredMouseDownCallback, handleDeselection, eligibleToDeselect = false) => {
this.handleDeferredMouseDown = () => {
if (eligibleToDeselect) {
this.handleDeferredMouseDown = (wasClicked) => {
if (wasClicked && eligibleToDeselect) {
handleDeselection();
}
handleDeferredMouseDownCallback();
Expand All @@ -566,7 +568,7 @@ export default observer(
};
this.resetDeferredClickTimeout();
this.deferredClickTimeout.push(setTimeout(() => {
this.handleDeferredMouseDown?.();
this.handleDeferredMouseDown?.(false);
}, this.props.item.annotation.isDrawing ? 0 : 100));
};

Expand All @@ -575,6 +577,7 @@ export default observer(
const isPanTool = item.getToolsManager().findSelectedTool()?.fullName === 'ZoomPanTool';
const isMoveTool = item.getToolsManager().findSelectedTool()?.fullName === 'MoveTool';

this.skipNextMouseDown = this.skipNextMouseUp = this.skipNextClick = false;
if (isFF(FF_LSDV_4930)) {
this.mouseDownPoint = { x: e.evt.offsetX, y: e.evt.offsetY };
}
Expand Down Expand Up @@ -626,6 +629,10 @@ export default observer(

this.canvasX = left;
this.canvasY = top;
if (this.skipNextMouseDown) {
this.skipNextMouseDown = false;
return true;
}
item.event('mousedown', e, x, y);

return true;
Expand All @@ -652,7 +659,9 @@ export default observer(

const handleDeselection = () => {
item.annotation.unselectAll();
this.skipMouseUp = true;
this.skipNextMouseDown = true;
this.skipNextMouseUp = true;
this.skipNextClick = true;
};

this.handleDeferredClick(handleMouseDown, handleDeselection, eligibleToDeselect);
Expand Down Expand Up @@ -680,7 +689,7 @@ export default observer(

item.freezeHistory();

return item.event('mouseup', e, x - this.canvasX, y - this.canvasY);
return this.triggerMouseUp(e, x - this.canvasX, y - this.canvasY);
};

handleGlobalMouseMove = e => {
Expand All @@ -705,7 +714,17 @@ export default observer(
item.freezeHistory();
item.setSkipInteractions(false);

return item.event('mouseup', e, e.evt.offsetX, e.evt.offsetY);
return this.triggerMouseUp(e, e.evt.offsetX, e.evt.offsetY);
};

triggerMouseUp = (e, x, y) => {
if (this.skipNextMouseUp) {
this.skipNextMouseUp = false;
return;
}
const { item } = this.props;

return item.event('mouseup', e, x, y);
};

handleMouseMove = e => {
Expand All @@ -721,7 +740,7 @@ export default observer(

if (isFF(FF_DEV_1442) && isDragging) {
this.resetDeferredClickTimeout();
this.handleDeferredMouseDown?.();
this.handleDeferredMouseDown?.(false);
}

if ((isMouseWheelClick || isShiftDrag) && item.zoomScale > 1) {
Expand Down
32 changes: 32 additions & 0 deletions tests/functional/data/image_segmentation/tools/rect.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
export const rectangleToolAndLabelsConfig = `<View>
<Image name="img" value="$image" />
<Rectangle name="rect" toName="img" />
<Labels name="labels" toName="img">
<Label value="Label 1" background="blue" />
<Label value="Label 2" background="red" />
</Labels>
</View>`;

export const simpleImageData = {
image: 'https://htx-misc.s3.amazonaws.com/opensource/label-studio/examples/images/nick-owuor-astro-nic-visuals-wDifg5xc9Z4-unsplash.jpg',
};

export const simpleRectangleResult = [
{
'id': 'rect_1',
'original_width': 2242,
'original_height': 2802,
'image_rotation': 0,
'value': {
'x': 20,
'y': 20,
'width': 20,
'height': 20,
'rotation': 0,
},
'from_name': 'rect',
'to_name': 'img',
'type': 'rectangle',
'origin': 'manual',
},
];
99 changes: 99 additions & 0 deletions tests/functional/specs/image_segmentation/tools/rect.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { ImageView, Labels, LabelStudio, Sidebar } from '@heartexlabs/ls-test/helpers/LSF';
import {
rectangleToolAndLabelsConfig,
simpleImageData,
simpleRectangleResult
} from 'data/image_segmentation/tools/rect';
import { FF_DEV_1442 } from '../../../../../src/utils/feature-flags';

describe('Rectangle tool', () => {
it('should not draw rectangle when clicking outside to unselect (FF_DEV_1442 = true)', () => {
LabelStudio.addFeatureFlagsOnPageLoad({
[FF_DEV_1442]: true,
});

LabelStudio.params()
.config(rectangleToolAndLabelsConfig)
.data(simpleImageData)
.withResult(simpleRectangleResult)
.init();

LabelStudio.waitForObjectsReady();

Sidebar.hasRegions(1);
Sidebar.toggleRegionSelection(0);
Labels.select('Label 1');

ImageView.clickAtRelative(0.5, 0.5);
ImageView.clickAtRelative(0.7, 0.7);
Sidebar.hasRegions(1);
Labels.selectedLabel.should('have.length', 0);
});

it('should draw rectangle by dragging (FF_DEV_1442 = true)', () => {
LabelStudio.addFeatureFlagsOnPageLoad({
[FF_DEV_1442]: true,
});

LabelStudio.params()
.config(rectangleToolAndLabelsConfig)
.data(simpleImageData)
.withResult(simpleRectangleResult)
.init();

LabelStudio.waitForObjectsReady();

Sidebar.hasRegions(1);
Sidebar.toggleRegionSelection(0);
Labels.select('Label 1');

ImageView.drawRectRelative(0.5, 0.5, 0.2, 0.2);
Sidebar.hasRegions(2);
Labels.selectedLabel.should('contain.text', 'Label 1');
});

it('should draw rectangle when clicking outside (FF_DEV_1442 = false)', () => {
LabelStudio.addFeatureFlagsOnPageLoad({
[FF_DEV_1442]: false,
});

LabelStudio.params()
.config(rectangleToolAndLabelsConfig)
.data(simpleImageData)
.withResult(simpleRectangleResult)
.init();

LabelStudio.waitForObjectsReady();

Sidebar.hasRegions(1);
Sidebar.toggleRegionSelection(0);
Labels.select('Label 1');

ImageView.clickAtRelative(0.5, 0.5);
ImageView.clickAtRelative(0.7, 0.7);
Sidebar.hasRegions(2);
Labels.selectedLabel.should('contain.text', 'Label 1');
});

it('should draw rectangle by dragging (FF_DEV_1442 = false)', () => {
LabelStudio.addFeatureFlagsOnPageLoad({
[FF_DEV_1442]: false,
});

LabelStudio.params()
.config(rectangleToolAndLabelsConfig)
.data(simpleImageData)
.withResult(simpleRectangleResult)
.init();

LabelStudio.waitForObjectsReady();

Sidebar.hasRegions(1);
Sidebar.toggleRegionSelection(0);
Labels.select('Label 1');

ImageView.drawRectRelative(0.5, 0.5, 0.2, 0.2);
Sidebar.hasRegions(2);
Labels.selectedLabel.should('contain.text', 'Label 1');
});
});

0 comments on commit 01a12b4

Please sign in to comment.