From 95e32687afe3b32ae84ccf858326e2aa0ae21bcc Mon Sep 17 00:00:00 2001 From: Vemparala Surya Vamsi <121419957+vsvamsi1@users.noreply.github.com> Date: Tue, 6 Aug 2024 17:19:22 +0530 Subject: [PATCH] chore: Memoising some computations within WDS_TOOLBAR_BUTTONS_WIDGET (#35436) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Memoising some computations related to WDS_TOOLBAR_BUTTONS_WIDGET Fixes #`Issue Number` ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: c15e274b718fc36f17b8f7edda4f23790b72f16d > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Tue, 06 Aug 2024 10:43:05 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit ## Summary by CodeRabbit - **Performance Enhancements** - Optimized the `ToolbarButtons` component for improved efficiency by utilizing the `useMemo` hook, reducing unnecessary calculations and re-renders. - Consolidated the computation of `sortedButtons` and `disabledKeys` in `ToolbarButtonsComponent` for better performance and clarity. - **Telemetry Integration** - Added performance tracking capabilities to the `withWidgetProps` higher-order component, enhancing observability of widget performance. --- .../ToolbarButtons/src/ToolbarButtons.tsx | 13 +++++--- .../component/index.tsx | 31 ++++++++++--------- app/client/src/widgets/withWidgetProps.tsx | 4 ++- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/app/client/packages/design-system/widgets/src/components/ToolbarButtons/src/ToolbarButtons.tsx b/app/client/packages/design-system/widgets/src/components/ToolbarButtons/src/ToolbarButtons.tsx index 4068e8872c..fd92ef4380 100644 --- a/app/client/packages/design-system/widgets/src/components/ToolbarButtons/src/ToolbarButtons.tsx +++ b/app/client/packages/design-system/widgets/src/components/ToolbarButtons/src/ToolbarButtons.tsx @@ -1,4 +1,4 @@ -import React, { forwardRef } from "react"; +import React, { forwardRef, useMemo } from "react"; import { Button, Menu } from "@design-system/widgets"; import { FocusScope } from "@react-aria/focus"; import { useDOMRef } from "@react-spectrum/utils"; @@ -37,11 +37,14 @@ const _ToolbarButtonsInner = ( domRef, ); - let children = [...state.collection]; - const menuChildren = (props.items as ToolbarButtonsItem[]).slice( - visibleItems, + const menuChildren = useMemo( + () => (props.items as ToolbarButtonsItem[]).slice(visibleItems), + [props.items, visibleItems], + ); + const children = useMemo( + () => [...state.collection].slice(0, visibleItems), + [state.collection, visibleItems], ); - children = children.slice(0, visibleItems); return ( diff --git a/app/client/src/widgets/wds/WDSToolbarButtonsWidget/component/index.tsx b/app/client/src/widgets/wds/WDSToolbarButtonsWidget/component/index.tsx index d3f06c2a14..d399232658 100644 --- a/app/client/src/widgets/wds/WDSToolbarButtonsWidget/component/index.tsx +++ b/app/client/src/widgets/wds/WDSToolbarButtonsWidget/component/index.tsx @@ -1,5 +1,5 @@ import type { Key } from "react"; -import React, { useState } from "react"; +import React, { useMemo, useState } from "react"; import { ToolbarButtons } from "@design-system/widgets"; import type { ToolbarButtonsComponentProps, @@ -23,21 +23,24 @@ export const ToolbarButtonsComponent = ( variant, } = props; - const sortedButtons = sortBy( - Object.keys(buttonsList) + const { disabledKeys, sortedButtons } = useMemo(() => { + const sortedButtons = sortBy( + Object.keys(buttonsList) + .map((key) => buttonsList[key]) + .filter((button) => { + return button.isVisible === true; + }), + ["index"], + ); + + const disabledKeys = Object.keys(buttonsList) .map((key) => buttonsList[key]) .filter((button) => { - return button.isVisible === true; - }), - ["index"], - ); - - const disabledKeys = Object.keys(buttonsList) - .map((key) => buttonsList[key]) - .filter((button) => { - return button.isDisabled; - }) - .map((button) => button.id); + return button.isDisabled; + }) + .map((button) => button.id); + return { sortedButtons, disabledKeys }; + }, [buttonsList]); const onActionComplete = (button: ToolbarButtonsItemComponentProps) => { const newLoadingButtonIds = [...loadingButtonIds]; diff --git a/app/client/src/widgets/withWidgetProps.tsx b/app/client/src/widgets/withWidgetProps.tsx index 0abe1b3806..e8b318375e 100644 --- a/app/client/src/widgets/withWidgetProps.tsx +++ b/app/client/src/widgets/withWidgetProps.tsx @@ -51,6 +51,7 @@ import WidgetFactory from "WidgetProvider/factory"; import { getIsAnvilLayout } from "layoutSystems/anvil/integrations/selectors"; import { WidgetProfiler } from "./BaseWidgetHOC/WidgetProfiler"; import { getAppsmithConfigs } from "@appsmith/configs"; +import { endSpan, startRootSpan } from "UITelemetry/generateTraces"; const { newRelic } = getAppsmithConfigs(); const WIDGETS_WITH_CHILD_WIDGETS = ["LIST_WIDGET", "FORM_WIDGET"]; @@ -69,6 +70,7 @@ function withWidgetProps(WrappedWidget: typeof BaseWidget) { widgetId, } = props; + const span = startRootSpan("withWidgetProps", { widgetType: type }); const isPreviewMode = useSelector(combinedPreviewModeSelector); const canvasWidget = useSelector((state: AppState) => @@ -253,7 +255,7 @@ function withWidgetProps(WrappedWidget: typeof BaseWidget) { // adding google maps api key to widget props (although meant for map widget only) widgetProps.googleMapsApiKey = googleMapsApiKey; - + endSpan(span); // isVisible prop defines whether to render a detached widget if ( widgetProps.detachFromLayout &&