Skip to content
This repository was archived by the owner on Apr 18, 2024. It is now read-only.

Commit cc058c9

Browse files
Gondragosbmartel
andauthored
fix: LSDV-4930: Fix selection tool (#1331)
* fix: LSDV-4930: Fix selection tool - stop selecting hidden regions - fix onClick issue in Konva - allow to interact with regions inside selection area * test: LSDV-4930: Add integration tests for selection tool * fix old tests * Fix ff applying * Update yarn.lock * Fix yarn lock * Update @heartexlabs/ls-test * Move tests data to separated file * fix: LSDV-4930: Fix selecting area after region has been created This is fix for ff_front_dev_1442_unselect_shape_on_click_outside_080622_short * Update testing library version * Add addFeatureFlagsOnPageLoad self-check test * test: LSDV-4930: Add test for selecting area after region has been created This is test for ff_front_dev_1442_unselect_shape_on_click_outside_080622_short * fix: LSDV-4930: Fix deselection functionality * Remove 'only' modifier of the test * Update @heartexlabs/ls-test * Update @heartexlabs/ls-test * Update @heartexlabs/ls-test * Fix formatting and add clarify comments * Add test for selecting one region from group of selected regions * Update @heartexlabs/ls-test * Update @heartexlabs/ls-test --------- Co-authored-by: bmartel <[email protected]>
1 parent 93f2982 commit cc058c9

File tree

13 files changed

+669
-14
lines changed

13 files changed

+669
-14
lines changed

e2e/setup/feature-flags.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ module.exports = {
77
fflag_feat_front_lsdv_4712_skipduplicates_editing_110423_short: true,
88
fflag_fix_font_lsdv_1148_hotkeys_namespaces_01022023_short: true,
99
fflag_fix_front_lsdv_4881_timeseries_points_missing_140423_short: true,
10+
fflag_fix_front_lsdv_4930_selection_tool_fixes_240423_short: true,
1011
};

e2e/tests/helpers.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,9 +363,15 @@ const polygonKonva = async (points) => {
363363
const stage = window.Konva.stages[0];
364364

365365
for (const point of points) {
366+
stage.fire('mousedown', {
367+
evt: { offsetX: point[0], offsetY: point[1], timeStamp: Date.now(), preventDefault: () => {} },
368+
});
366369
stage.fire('click', {
367370
evt: { offsetX: point[0], offsetY: point[1], timeStamp: Date.now(), preventDefault: () => {} },
368371
});
372+
stage.fire('mouseup', {
373+
evt: { offsetX: point[0], offsetY: point[1], timeStamp: Date.now(), preventDefault: () => {} },
374+
});
369375
await delay(50);
370376
}
371377

src/components/ImageView/ImageView.js

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,17 @@ import ResizeObserver from '../../utils/resize-observer';
2020
import { debounce } from '../../utils/debounce';
2121
import Constants from '../../core/Constants';
2222
import { fixRectToFit } from '../../utils/image';
23-
import { FF_DEV_1285, FF_DEV_1442, FF_DEV_3077, FF_DEV_3793, FF_DEV_4081, FF_LSDV_4583_6, FF_LSDV_4711, isFF } from '../../utils/feature-flags';
23+
import {
24+
FF_DEV_1285,
25+
FF_DEV_1442,
26+
FF_DEV_3077,
27+
FF_DEV_3793,
28+
FF_DEV_4081,
29+
FF_LSDV_4583_6,
30+
FF_LSDV_4711,
31+
FF_LSDV_4930,
32+
isFF
33+
} from '../../utils/feature-flags';
2434
import { Pagination } from '../../common/Pagination/Pagination';
2535
import { Image } from './Image';
2636

@@ -89,7 +99,7 @@ const Regions = memo(({ regions, useLayers = true, chunkSize = 15, suggestion =
8999

90100
const DrawingRegion = observer(({ item }) => {
91101
const { drawingRegion } = item;
92-
102+
93103
if (!drawingRegion) return null;
94104
if (item.multiImage && item.currentImage !== drawingRegion.item_index) return null;
95105

@@ -280,7 +290,11 @@ const SelectedRegions = observer(({ item, selectedRegions }) => {
280290

281291
return (
282292
<>
283-
<TransformerBack item={item}/>
293+
{
294+
isFF(FF_LSDV_4930)
295+
? null
296+
: <TransformerBack item={item} />
297+
}
284298
{brushRegions.length > 0 && (
285299
<Regions
286300
key="brushes"
@@ -326,7 +340,7 @@ const SelectionLayer = observer(({ item, selectionArea }) => {
326340
window.removeEventListener('mousedown', dragHandler);
327341
window.removeEventListener('mouseup', dragHandler);
328342
};
329-
},[]);
343+
}, []);
330344

331345
const disableTransform = item.zoomScale > 1 && (shift || isPanTool || isMouseWheelClick);
332346

@@ -375,7 +389,7 @@ const Selection = observer(({ item, selectionArea, ...triggeredOnResize }) => {
375389
return (
376390
<>
377391
<SelectedRegions item={item} selectedRegions={item.selectedRegions} {...triggeredOnResize} />
378-
<SelectionLayer item={item} selectionArea={selectionArea}/>
392+
<SelectionLayer item={item} selectionArea={selectionArea} />
379393
</>
380394
);
381395
});
@@ -475,6 +489,7 @@ export default observer(
475489
handleDeferredMouseDown = null;
476490
deferredClickTimeout = [];
477491
skipMouseUp = false;
492+
mouseDownPoint = null;
478493

479494
constructor(props) {
480495
super(props);
@@ -495,8 +510,22 @@ export default observer(
495510
}
496511

497512
const evt = e.evt || e;
513+
const { offsetX: x, offsetY: y } = evt;
498514

499-
return item.event('click', evt, evt.offsetX, evt.offsetY);
515+
if (isFF(FF_LSDV_4930)) {
516+
// Konva can trigger click even on simple mouseup
517+
// You can try drag and drop interaction here https://konvajs.org/docs/events/Stage_Events.html and check the console
518+
// So here is false trigger preventing
519+
if (
520+
!this.mouseDownPoint
521+
|| Math.abs(this.mouseDownPoint.x - x) > 0.01
522+
|| Math.abs(this.mouseDownPoint.y - y) > 0.01
523+
) {
524+
this.mouseDownPoint = null;
525+
return;
526+
}
527+
}
528+
return item.event('click', evt, x, y);
500529
};
501530

502531
resetDeferredClickTimeout = () => {
@@ -514,6 +543,8 @@ export default observer(
514543
handleDeselection();
515544
}
516545
handleDeferredMouseDownCallback();
546+
// mousedown should be called only once especially if it is called from mousemove interaction.
547+
this.handleDeferredMouseDown = null;
517548
};
518549
this.resetDeferredClickTimeout();
519550
this.deferredClickTimeout.push(setTimeout(() => {
@@ -525,6 +556,10 @@ export default observer(
525556
const { item } = this.props;
526557
const isPanTool = item.getToolsManager().findSelectedTool()?.fullName === 'ZoomPanTool';
527558

559+
if (isFF(FF_LSDV_4930)) {
560+
this.mouseDownPoint = { x: e.evt.offsetX, y: e.evt.offsetY };
561+
}
562+
528563
item.updateSkipInteractions(e);
529564

530565
const p = e.target.getParent();
@@ -534,7 +569,7 @@ export default observer(
534569

535570
const handleMouseDown = () => {
536571
if (
537-
// create regions over another regions with Cmd/Ctrl pressed
572+
// create regions over another regions with Cmd/Ctrl pressed
538573
item.getSkipInteractions() ||
539574
e.target === item.stageRef ||
540575
findClosestParent(
@@ -1042,6 +1077,12 @@ export default observer(
10421077
)}
10431078
{item.grid && item.sizeUpdated && <ImageGrid item={item} />}
10441079

1080+
{
1081+
isFF(FF_LSDV_4930)
1082+
? <TransformerBack item={item} />
1083+
: null
1084+
}
1085+
10451086
{renderableRegions.map(([groupName, list]) => {
10461087
const isBrush = groupName.match(/brush/i) !== null;
10471088
const isSuggestion = groupName.match('suggested') !== null;

src/mixins/AreaMixin.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { guidGenerator } from '../core/Helpers';
44
import Result from '../regions/Result';
55
import { PER_REGION_MODES } from './PerRegion';
66
import { ReadOnlyRegionMixin } from './ReadOnlyMixin';
7+
import { FF_LSDV_4930, isFF } from '../utils/feature-flags';
78

89
let ouid = 1;
910

@@ -124,7 +125,8 @@ export const AreaMixinBase = types
124125
},
125126

126127
get isInSelectionArea() {
127-
return self.parent?.selectionArea?.isActive ? self.parent.selectionArea.intersectsBbox(self.bboxCoords) : false;
128+
return (!isFF(FF_LSDV_4930) || !self.hidden)
129+
&& self.parent?.selectionArea?.isActive ? self.parent.selectionArea.intersectsBbox(self.bboxCoords) : false;
128130
},
129131
}))
130132
.volatile(() => ({

src/mixins/DrawingTool.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,8 @@ const TwoPointsDrawingTool = DrawingTool.named('TwoPointsDrawingTool')
301301

302302
clickEv(_, [x, y]) {
303303
if (!self.canStartDrawing()) return;
304+
// @todo: here is a potential problem with endPoint
305+
// it may be incorrect due to it may be not set at this moment
304306
if (startPoint && endPoint && !self.comparePointsWithThreshold(startPoint, endPoint)) return;
305307
if (currentMode === DEFAULT_MODE) {
306308
modeAfterMouseMove = TWO_CLICKS_MODE;

src/tools/Selection.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import BaseTool from './Base';
44
import ToolMixin from '../mixins/Tool';
55
import { AnnotationMixin } from '../mixins/AnnotationMixin';
66
import { IconMoveTool } from '../assets/icons';
7+
import { FF_LSDV_4930, isFF } from '../utils/feature-flags';
78

89
const _Tool = types.model('SelectionTool', {
910
shortcut: 'V',
@@ -54,6 +55,15 @@ const _Tool = types.model('SelectionTool', {
5455
}
5556
isSelecting = false;
5657
},
58+
clickEv(ev) {
59+
if (isFF(FF_LSDV_4930)) {
60+
isSelecting = false;
61+
self.obj.resetSelection();
62+
if (!ev.ctrlKey && !ev.metaKey) {
63+
self.annotation.unselectAreas();
64+
}
65+
}
66+
},
5767
};
5868
});
5969

src/utils/feature-flags.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,13 @@ export const FF_LSDV_4832 = 'fflag_feat_front_lsdv_4832_new_ranker_tag_120423_sh
240240
*/
241241
export const FF_LSDV_4881 = 'fflag_fix_front_lsdv_4881_timeseries_points_missing_140423_short';
242242

243+
/**
244+
* Fixing issues related to selection tool functional (selecting hidden regions, onClick in Konva, interaction with regions inside selection area)
245+
*
246+
* @link https://app.launchdarkly.com/default/production/features/fflag_fix_front_lsdv_4930_selection_tool_fixes_240423_short
247+
*/
248+
export const FF_LSDV_4930 = 'fflag_fix_front_lsdv_4930_selection_tool_fixes_240423_short';
249+
243250
/**
244251
* Restore "hide all regions" button functionality in the outliner
245252
*

0 commit comments

Comments
 (0)