From 3c1f5fdfb38e64a18410ab0b160aa4910b6b6d9a Mon Sep 17 00:00:00 2001 From: rahulramesha <71900764+rahulramesha@users.noreply.github.com> Date: Fri, 2 Jun 2023 17:36:00 +0530 Subject: [PATCH] chore: Fix Style recalcs for stickyCanvasArena (#22904) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Refactor sticky canvas Arena to avoid unnecessary forced reflow. This is done by enabling the canvas to observe only when required i.e, only when user is using, - Drag to select - Dragging widgets Fixes #23777 Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video ## Type of change - Chore (housekeeping or task changes that don't impact user perception) ## How Has This Been Tested? - Manual ### 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 --- .../CanvasArenas/CanvasDraggingArena.tsx | 7 ++++ .../CanvasArenas/CanvasSelectionArena.tsx | 1 + .../common/CanvasArenas/StickyCanvasArena.tsx | 40 +++++++++++++------ 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/app/client/src/pages/common/CanvasArenas/CanvasDraggingArena.tsx b/app/client/src/pages/common/CanvasArenas/CanvasDraggingArena.tsx index cc678e8487..5a32e9c998 100644 --- a/app/client/src/pages/common/CanvasArenas/CanvasDraggingArena.tsx +++ b/app/client/src/pages/common/CanvasArenas/CanvasDraggingArena.tsx @@ -1,7 +1,9 @@ +import type { AppState } from "@appsmith/reducers"; import { theme } from "constants/DefaultTheme"; import { MAIN_CONTAINER_WIDGET_ID } from "constants/WidgetConstants"; import React, { useMemo } from "react"; import { useSelector } from "react-redux"; +import { isCurrentCanvasDragging } from "sagas/selectors"; import { getIsAutoLayout } from "selectors/editorSelectors"; import type { LayoutDirection } from "utils/autoLayout/constants"; import { getNearestParentCanvas } from "utils/generators"; @@ -70,6 +72,10 @@ export function CanvasDraggingArena({ stickyCanvasRef, slidingArenaRef, }); + + const isDragging = useSelector((state: AppState) => + isCurrentCanvasDragging(state, widgetId), + ); return showCanvas ? ( Element | null; canExtend: boolean; ref: StickyCanvasArenaRef; + shouldObserveIntersection: boolean; } interface StickyCanvasArenaRef { @@ -43,6 +44,7 @@ export const StickyCanvasArena = forwardRef( canvasPadding, getRelativeScrollingParent, id, + shouldObserveIntersection, showCanvas, snapColSpace, snapRows, @@ -55,6 +57,7 @@ export const StickyCanvasArena = forwardRef( entries.forEach(updateCanvasStylesIntersection); }), ); + const resizeObserver = useRef( new ResizeObserver(() => { observeSlider(); @@ -88,20 +91,26 @@ export const StickyCanvasArena = forwardRef( entry: IntersectionObserverEntry, ) => { if (slidingArenaRef.current) { - const parentCanvas: Element | null = getRelativeScrollingParent( - slidingArenaRef.current, - ); + requestAnimationFrame(() => { + const parentCanvas: Element | null = getRelativeScrollingParent( + slidingArenaRef.current, + ); - if (parentCanvas && stickyCanvasRef.current) { - repositionSliderCanvas(entry); - rescaleSliderCanvas(entry); - } + if (parentCanvas && stickyCanvasRef.current) { + repositionSliderCanvas(entry); + rescaleSliderCanvas(entry); + } + }); } }; + const observeSlider = () => { - interSectionObserver.current.disconnect(); - if (slidingArenaRef && slidingArenaRef.current) { - interSectionObserver.current.observe(slidingArenaRef.current); + // This is to make sure the canvas observes and changes only when needed like when dragging or drw to select. + if (shouldObserveIntersection) { + interSectionObserver.current.disconnect(); + if (slidingArenaRef && slidingArenaRef.current) { + interSectionObserver.current.observe(slidingArenaRef.current); + } } }; @@ -109,7 +118,14 @@ export const StickyCanvasArena = forwardRef( if (slidingArenaRef.current) { observeSlider(); } - }, [showCanvas, snapRows, canExtend, snapColSpace, snapRowSpace]); + }, [ + showCanvas, + snapRows, + canExtend, + snapColSpace, + snapRowSpace, + shouldObserveIntersection, + ]); useEffect(() => { let parentCanvas: Element | null; @@ -126,7 +142,7 @@ export const StickyCanvasArena = forwardRef( resizeObserver.current.unobserve(slidingArenaRef.current); } }; - }, []); + }, [shouldObserveIntersection]); return ( <> {/* Canvas will always be sticky to its scrollable parent's view port. i.e,