Skip to content

Commit

Permalink
perf: Anvil widget name component rendering (#33672)
Browse files Browse the repository at this point in the history
## Description
- Fixes some of the performance issues in the widget name component for
Anvil. Particularly, the following
- All widget name components for all widgets used to be added to the DOM
tree, even if they're not visible
- All widget name components had the floating-ui listeners added to them
at all times, leading to a sharp rise in the number of JS listeners and
increasing RAM usage.
- Widget name component positions used to recompute sporadically,
leading to perceived jitter when users scrolled too quickly.

The above issues have been fixed by the following changes
- Widget name component gets added to the DOM only when
`nameComponentState !== "none"`
- Widget name component only has listeners added by floating-ui when the
name component needs to be visible
- Floating-ui is configured to recompute in all `requestAnimationFrame`
cycles


Fixes #33386
Fixes #33330 

## Automation

/ok-to-test tags="@tag.All"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!CAUTION]
> 🔴 🔴 🔴 Some tests have failed.
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/9205700153>
> Commit: db2487d
> Cypress dashboard: <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9205700153&attempt=1&selectiontype=test&testsstatus=failed&specsstatus=fail"
target="_blank"> Click here!</a>
> The following are new failures, please fix them before merging the PR:
<ol>
> <li>cypress/e2e/Regression/ClientSide/BugTests/GitBugs_Spec.ts </ol>
> To know the list of identified flaky tests - <a
href="https://internal.appsmith.com/app/cypress-dashboard/identified-flaky-tests-65890b3c81d7400d08fa9ee3?branch=master"
target="_blank">Refer here</a>

<!-- end of auto-generated comment: Cypress test results  -->





## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No
  • Loading branch information
riodeuno committed May 23, 2024
1 parent 9a9e97c commit e0df8f7
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,13 @@ export function AnvilWidgetName(props: {

let cleanup = () => {};
useEffect(() => {
if (widgetElement && widgetNameComponent && widgetsEditorElement) {
if (
widgetElement &&
widgetNameComponent &&
widgetsEditorElement &&
// Makes sure we add listeners only if the widget is selected or focused
nameComponentState !== "none"
) {
cleanup = handleWidgetUpdate(
widgetElement,
widgetNameComponent,
Expand All @@ -108,6 +114,8 @@ export function AnvilWidgetName(props: {
return null;
// Don't show widget name component if the widget DOM element isn't found
if (!widgetElement) return null;
// Don't render any DOM nodes if the widget is not selected or focused
if (nameComponentState === "none") return null;

return (
<FloatingPortal>
Expand Down
60 changes: 32 additions & 28 deletions app/client/src/layoutSystems/anvil/editor/AnvilWidgetName/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,38 +65,42 @@ export function handleWidgetUpdate(
widgetsEditorElement: HTMLDivElement,
nameComponentState: NameComponentStates,
) {
return autoUpdate(widgetElement, widgetNameComponent, () => {
computePosition(widgetElement as HTMLDivElement, widgetNameComponent, {
placement: "top-start",
strategy: "fixed",
middleware: [
flip(),
shift(),
offset({ mainAxis: 0, crossAxis: -5 }),
getOverflowMiddleware(widgetsEditorElement as HTMLDivElement),
hide({ strategy: "referenceHidden" }),
hide({ strategy: "escaped" }),
],
}).then(({ middlewareData, x, y }) => {
let shiftOffset = 0;
if (middlewareData.containWithinCanvas.overflowAmount > 0) {
shiftOffset = middlewareData.containWithinCanvas.overflowAmount + 5;
}
return autoUpdate(
widgetElement,
widgetNameComponent,
() => {
computePosition(widgetElement as HTMLDivElement, widgetNameComponent, {
placement: "top-start",
strategy: "fixed",
middleware: [
flip(),
shift(),
offset({ mainAxis: 0, crossAxis: -5 }),
getOverflowMiddleware(widgetsEditorElement as HTMLDivElement),
hide({ strategy: "referenceHidden" }),
hide({ strategy: "escaped" }),
],
}).then(({ middlewareData, x, y }) => {
let shiftOffset = 0;
if (middlewareData.containWithinCanvas.overflowAmount > 0) {
shiftOffset = middlewareData.containWithinCanvas.overflowAmount + 5;
}

Object.assign(widgetNameComponent.style, {
left: `${x - shiftOffset}px`,
top: `${y}px`,
visibility:
nameComponentState === "none" || middlewareData.hide?.referenceHidden
Object.assign(widgetNameComponent.style, {
left: `${x - shiftOffset}px`,
top: `${y}px`,
visibility: middlewareData.hide?.referenceHidden
? "hidden"
: "visible",
zIndex:
nameComponentState === "focus"
? "calc(var(--on-canvas-ui-zindex) + 1)"
: "var(--on-canvas-ui-zindex)",
zIndex:
nameComponentState === "focus"
? "calc(var(--on-canvas-ui-zindex) + 1)"
: "var(--on-canvas-ui-zindex)",
});
});
});
});
},
{ animationFrame: true },
);
}

export function getWidgetNameComponentStyleProps(
Expand Down

0 comments on commit e0df8f7

Please sign in to comment.