fix: Drag to select widgets within container like widgets ends up selecting Parent widget (#20885)

## Description
This PR fixes, drag to select widget feature inside container like
widgets. The changes include,

- Add condition to stop triggering select action when, drag to select is
still active
- Delay stopping drag to select to the end of the execution stack, to
prevent triggering selection action
- Change name of `isDragging` to `isMouseDown` to avoid confusion with
the other `isDragging` in the same file
- Trigger start dragging to select action after `mousedown` and
`mousemove` instead on every `mousedown`


Fixes #20804 


Media



## Type of change

- Bug fix (non-breaking change which fixes an issue)

## How Has This Been Tested?

- Manual
Before


https://user-images.githubusercontent.com/71900764/220725344-a4a50770-1335-405f-ac32-2ec63d3c9e6f.mp4

After


https://user-images.githubusercontent.com/71900764/220725390-9d94cd31-28d2-4b21-ae62-dbb98c2678ea.mp4



### Test Plan
> Add Testsmith test cases links that relate to this PR

### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)


## Checklist:
### Dev activity
- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


### QA activity:
- [ ] Test plan has been approved by relevant developers
- [ ] Test plan has been peer reviewed by QA
- [ ] Cypress test cases have been added and approved by either SDET or
manual QA
- [ ] Organized project review call with relevant stakeholders after
Round 1/2 of QA
- [ ] Added Test Plan Approved label after reveiwing all Cypress test
This commit is contained in:
rahulramesha 2023-03-07 15:42:15 +05:30 committed by GitHub
parent c3c6218c0f
commit a66d64b547
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 41 additions and 10 deletions

View File

@ -44,6 +44,11 @@ describe("Canvas selection test cases", () => {
beforeEach(() => {
spyWidgetSelection.mockClear();
jest.useFakeTimers();
});
afterEach(() => {
jest.useRealTimers();
});
it("Should select using canvas draw", () => {
@ -502,6 +507,8 @@ describe("Canvas selection test cases", () => {
`canvas-selection-${MAIN_CONTAINER_WIDGET_ID}`,
);
jest.runOnlyPendingTimers();
expect(selectionCanvas.style.zIndex).toBe("");
act(() => {
fireEvent.dragStart(widgetEditor);

View File

@ -169,7 +169,8 @@ export function CanvasSelectionArena({
});
let selectionRectangle: SelectedArenaDimensions = initRectangle();
let isMultiSelect = false;
let isDragging = false;
let isMouseDown = false;
let shouldStartCanvasDragging = false;
const getSelectionDimensions = () => {
return {
@ -242,7 +243,7 @@ export function CanvasSelectionArena({
const onMouseEnter = (e: any) => {
if (
slidingArenaRef.current &&
!isDragging &&
!isMouseDown &&
drawOnEnterObj?.current.canDraw
) {
firstRender(e, true);
@ -259,7 +260,7 @@ export function CanvasSelectionArena({
Math.abs(selectionRectangle.width) >
0
) {
if (!isDragging) {
if (!isMouseDown) {
// cant set this in onMouseUp coz click seems to happen after onMouseUp.
selectionRectangle = initRectangle();
}
@ -306,7 +307,11 @@ export function CanvasSelectionArena({
};
const firstRender = (e: any, fromOuterCanvas = false) => {
if (slidingArenaRef.current && stickyCanvasRef.current && !isDragging) {
if (
slidingArenaRef.current &&
stickyCanvasRef.current &&
!isMouseDown
) {
isMultiSelect = e.ctrlKey || e.metaKey || e.shiftKey;
if (fromOuterCanvas) {
const { left, top } = startPositionsForOutCanvasSelection();
@ -320,7 +325,8 @@ export function CanvasSelectionArena({
}
selectionRectangle.width = 0;
selectionRectangle.height = 0;
isDragging = true;
isMouseDown = true;
shouldStartCanvasDragging = true;
// bring the canvas to the top layer
stickyCanvasRef.current.style.zIndex = "2";
slidingArenaRef.current.style.zIndex = "2";
@ -335,13 +341,12 @@ export function CanvasSelectionArena({
slidingArenaRef.current &&
(!isDraggableParent || e.ctrlKey || e.metaKey)
) {
dispatch(setCanvasSelectionStateAction(true, widgetId));
firstRender(e);
}
};
const onMouseUp = () => {
if (isDragging && slidingArenaRef.current && stickyCanvasRef.current) {
isDragging = false;
if (isMouseDown && slidingArenaRef.current && stickyCanvasRef.current) {
isMouseDown = false;
canvasCtx.clearRect(
0,
0,
@ -351,11 +356,21 @@ export function CanvasSelectionArena({
stickyCanvasRef.current.style.zIndex = "";
slidingArenaRef.current.style.zIndex = "";
slidingArenaRef.current.style.cursor = "";
dispatch(setCanvasSelectionStateAction(false, widgetId));
//moving triggering action to the end of queue,
// to avoid selecting the widget being dragged on
setTimeout(() => {
dispatch(setCanvasSelectionStateAction(false, widgetId));
}, 0);
}
};
const onMouseMove = (e: any) => {
if (isDragging && slidingArenaRef.current && stickyCanvasRef.current) {
if (isMouseDown && slidingArenaRef.current && stickyCanvasRef.current) {
// This is to make sure we start selection only after dragging start
// rather than mouse down
if (shouldStartCanvasDragging) {
dispatch(setCanvasSelectionStateAction(true, widgetId));
shouldStartCanvasDragging = false;
}
selectionRectangle.width =
e.offsetX -
slidingArenaRef.current.offsetLeft -

View File

@ -148,6 +148,7 @@ export const shouldWidgetIgnoreClicksSelector = (widgetId: string) => {
getIsTableFilterPaneVisible,
(state: AppState) => state.ui.widgetDragResize.isResizing,
(state: AppState) => state.ui.widgetDragResize.isDragging,
(state: AppState) => state.ui.canvasSelection.isDraggingForSelection,
getAppMode,
getIsAutoHeightWithLimitsChanging,
(
@ -155,12 +156,14 @@ export const shouldWidgetIgnoreClicksSelector = (widgetId: string) => {
isTableFilterPaneVisible,
isResizing,
isDragging,
isDraggingForSelection,
appMode,
isAutoHeightWithLimitsChanging,
) => {
const isFocused = focusedWidgetId === widgetId;
return (
isDraggingForSelection ||
isResizing ||
isDragging ||
appMode !== APP_MODE.EDIT ||

View File

@ -40,6 +40,9 @@ describe("<DividerWidget />", () => {
autoHeightUI: {
isAutoHeightWithLimitsChanging: false,
},
canvasSelection: {
isDraggingForSelection: false,
},
},
entities: { canvasWidgets: {}, app: { mode: "canvas" } },
};

View File

@ -44,6 +44,9 @@ describe("<DropdownWidget />", () => {
autoHeightUI: {
isAutoHeightWithLimitsChanging: false,
},
canvasSelection: {
isDraggingForSelection: false,
},
},
entities: { canvasWidgets: {}, app: { mode: "canvas" } },
};