fix: infinite render loop in List Widget when hiding/unhiding with selected item (#39262)
## Description
When a List widget has a selected item and is hidden then unhidden
(unmounted/mounted), an infinite render loop occurs in the following
flow:
```typescript
// Triggered in componentDidUpdate
if (this.shouldUpdateSelectedItemAndView() && isString(this.props.selectedItemKey)) {
this.updateSelectedItemAndPageOnResetOrMount();
}
```
### Root Cause
This happens because:
1. `updateSelectedItemAndPageOnResetOrMount` calls `updatePageNumber`.
2. `updatePageNumber` uses `calculatePageNumberFromRowIndex`, which
depends on `this.pageSize`.
3. Each page number calculation triggers a meta property update through
`onPageChange`.
4. This meta update causes a rerender, repeating the cycle.
#### Problematic Calculation
`this.pageSize` is calculated dynamically based on widget dimensions:
```typescript
calculatePageNumberFromRowIndex = (index: number) => {
return Math.ceil((index + 1) / this.pageSize); // Using unstable this.pageSize
};
```
This creates instability during mount/unmount cycles as:
- Widget dimensions may not be fully stabilized.
- Each meta property update triggers recalculation.
- No break in the update-rerender cycle.
### Solution
Use `this.props.pageSize` instead of `this.pageSize` in page
calculations:
```typescript
calculatePageNumberFromRowIndex = (index: number) => {
return Math.ceil((index + 1) / this.props.pageSize); // Using stable props value
};
```
### Why This Works
- `this.props.pageSize` represents the last stable value set through the
meta property system.
- It's protected by the existing `pageSizeUpdated` flag mechanism.
- Breaking the dependency on dynamically calculated dimensions prevents
the infinite loop.
Fixes #37683
## Automation
/ok-to-test tags="@tag.Widget, @tag.Binding, @tag.List, @tag.Sanity"
### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/13366216706>
> Commit: 74028ec9f52aa25daf0d72c7cdf3c4fa9701e86e
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13366216706&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Widget, @tag.Binding, @tag.List, @tag.Sanity`
> Spec:
> <hr>Mon, 17 Feb 2025 10:23:19 UTC
<!-- end of auto-generated comment: Cypress test results -->
## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Bug Fixes**
- Improved the list widget’s pagination by updating how page numbers are
calculated, ensuring accurate reflection of the current page size
settings.
- **Tests**
- Enhanced test coverage for the list widget's visibility, adding new
tests to validate user interactions and ensuring the widget behaves as
expected when items are selected.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
5b9153cb19
commit
c0e073efa3
|
|
@ -35,7 +35,58 @@ describe(
|
|||
deployMode.NavigateBacktoEditor();
|
||||
});
|
||||
|
||||
it("2. Toggle JS - Validate isVisible", function () {
|
||||
it("2. Validate isVisible when list has selected item (#37683)", () => {
|
||||
// Define selectors for widgets
|
||||
const widgetSelector = (name: string) => `[data-widgetname-cy="${name}"]`;
|
||||
const containerWidgetSelector = `[type="CONTAINER_WIDGET"]`;
|
||||
|
||||
// Drag and drop the Button widget onto the canvas
|
||||
// entityExplorer.DragDropWidgetNVerify("listwidgetv2", 300, 300);
|
||||
entityExplorer.DragDropWidgetNVerify("buttonwidget");
|
||||
|
||||
// Set up the button to make the list visible when clicked
|
||||
EditorNavigation.SelectEntityByName("Button1", EntityType.Widget);
|
||||
propPane.EnterJSContext("onClick", `{{List1.setVisibility(true)}}`, true);
|
||||
|
||||
// Set up the list widget to become invisible when an item is clicked
|
||||
EditorNavigation.SelectEntityByName("List1", EntityType.Widget);
|
||||
propPane.EnterJSContext(
|
||||
"onItemClick",
|
||||
`{{List1.setVisibility(false)}}`,
|
||||
true,
|
||||
);
|
||||
|
||||
// Deploy the application to test the visibility functionality
|
||||
deployMode.DeployApp();
|
||||
|
||||
// Simulate a click on the first item in the list to hide the list
|
||||
agHelper
|
||||
.GetElement(widgetSelector("List1"))
|
||||
.find(containerWidgetSelector)
|
||||
.first()
|
||||
.click({ force: true });
|
||||
agHelper.WaitUntilEleDisappear(
|
||||
locators._widgetInDeployed(draggableWidgets.LIST_V2),
|
||||
);
|
||||
|
||||
// Assert that the list widget is not visible after clicking an item
|
||||
agHelper.AssertElementAbsence(
|
||||
locators._widgetInDeployed(draggableWidgets.LIST_V2),
|
||||
);
|
||||
|
||||
// Click the button to make the list visible again
|
||||
agHelper.GetNClick(widgetSelector("Button1"));
|
||||
|
||||
// Assert that the list widget is visible after clicking the button
|
||||
agHelper.AssertElementVisibility(
|
||||
locators._widgetInDeployed(draggableWidgets.LIST_V2),
|
||||
);
|
||||
|
||||
// Navigate back to the editor after testing
|
||||
deployMode.NavigateBacktoEditor();
|
||||
});
|
||||
|
||||
it("3. Toggle JS - Validate isVisible", function () {
|
||||
// Open Property pane
|
||||
agHelper.AssertElementVisibility(
|
||||
locators._widgetInDeployed(draggableWidgets.LIST_V2),
|
||||
|
|
@ -61,7 +112,7 @@ describe(
|
|||
deployMode.NavigateBacktoEditor();
|
||||
});
|
||||
|
||||
it("3. Renaming the widget from Property pane and Entity explorer ", function () {
|
||||
it("4. Renaming the widget from Property pane and Entity explorer ", function () {
|
||||
// Open Property pane
|
||||
EditorNavigation.SelectEntityByName("List1", EntityType.Widget);
|
||||
// Change the list widget name from property pane and Verify it
|
||||
|
|
@ -73,7 +124,7 @@ describe(
|
|||
agHelper.AssertElementVisibility(locators._widgetName("List1"));
|
||||
});
|
||||
|
||||
it("4. Item Spacing Validation ", function () {
|
||||
it("5. Item Spacing Validation ", function () {
|
||||
EditorNavigation.SelectEntityByName("List1", EntityType.Widget);
|
||||
propPane.Search("item spacing");
|
||||
propPane.UpdatePropertyFieldValue("Item Spacing (px)", "-1");
|
||||
|
|
|
|||
|
|
@ -878,7 +878,7 @@ class ListWidget extends BaseWidget<
|
|||
};
|
||||
|
||||
calculatePageNumberFromRowIndex = (index: number) => {
|
||||
return Math.ceil((index + 1) / this.pageSize);
|
||||
return Math.ceil((index + 1) / this.props.pageSize);
|
||||
};
|
||||
|
||||
shouldUpdatePageSize = () => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user