feat: Implement empty meta widget generation for handling empty data scenarios #40886
Conversation
…scenarios Added functionality to generate empty meta widgets when the list data is empty, preventing currentItem binding errors. This includes creating meta widgets with the same IDs as template widgets and ensuring safe currentItem bindings. Updated ListWidget to manage transitions between empty and non-empty data states effectively. - Introduced `generateEmptyMetaWidgets` method in MetaWidgetGenerator. - Enhanced ListWidget to track empty data state and generate meta widgets accordingly. - Added logic to reset meta widget cache when transitioning from empty to non-empty data. This improves the robustness of the ListWidget in handling dynamic data scenarios.
WalkthroughThe changes introduce a unified approach for handling empty list data in the ListWidgetV2 component. A new utility function determines if the list is fully empty on the first page, influencing how meta widgets are generated and rendered. The logic for empty data handling is now centralized and consistently applied across relevant modules. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant ListWidget
participant Helper as isListFullyEmpty
participant MetaWidgetGen
UI->>ListWidget: Render ListWidget (with listData, pageNo)
ListWidget->>Helper: isListFullyEmpty(listData, pageNo)
Helper-->>ListWidget: true/false
alt List is fully empty
ListWidget->>MetaWidgetGen: Generate meta widgets with [{}]
else
ListWidget->>MetaWidgetGen: Generate meta widgets with listData
end
MetaWidgetGen-->>ListWidget: Meta widgets
ListWidget-->>UI: Rendered widget view
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms (6)
🔇 Additional comments (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/15489666321. |
|
Deploy-Preview-URL: https://ce-40886.dp.appsmith.com |
…ta handling Eliminated the `generateEmptyMetaWidgets` method and related logic from the `MetaWidgetGenerator` and `ListWidget` components. The handling of empty data scenarios is now managed directly within the `generateMainMetaCanvasWidget` method, ensuring that an empty object is added to `listData` when it is empty. This simplifies the codebase and improves the efficiency of meta widget generation. - Removed redundant methods and properties related to empty meta widget generation. - Updated `ListWidget` to directly handle empty data cases without relying on separate meta widget generation logic. - Enhanced clarity and maintainability of the code by consolidating data handling logic.
…o rahulbarwal/fix-list-widget-issue
Updated the logic for handling empty data cases in the ListWidget and MetaWidgetGenerator. The check for empty data now considers the current page number to prevent generating empty meta widgets when navigating to a page with no data. Introduced a new helper function, isListFullyEmpty, to encapsulate this logic for better readability and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
app/client/src/widgets/ListWidgetV2/widget/index.tsx (1)
566-575: 🛠️ Refactor suggestionAvoid mutating
props.listData– clone instead
pushmutates the array that comes straight from props, violating React’s immutability expectations and risks subtle bugs when this method is called multiple times.- if (isListFullyEmpty(listData, pageNo)) { - listData.push({}); - } + const processedListData = isListFullyEmpty(listData, pageNo) + ? [{}] // placeholder for empty-data meta generation + : listData.slice(); // shallow clone keeps later logic untouchedand later use
processedListDatainstead oflistDatain thereturnobject.This avoids side-effects, keeps renders deterministic and prevents accidentally appending multiple placeholders if the binding re-evaluates to
[].
🧹 Nitpick comments (2)
app/client/src/widgets/ListWidgetV2/widget/helper.ts (1)
133-140: Guard against undefined inputs & favour immutability
isListFullyEmptywill throw iflistDataisundefined(e.g.listData?: unknown[]) and silently returnsfalsewhenpageNoisundefined. A tiny defensive tweak avoids both cases without changing semantics.-export const isListFullyEmpty = (listData: unknown[], pageNo: number) => { +export const isListFullyEmpty = ( + listData: unknown[] | undefined, + pageNo: number | undefined, +) => { ... - return listData.length === 0 && pageNo === 1; + return Array.isArray(listData) && listData.length === 0 && pageNo === 1; };app/client/src/widgets/ListWidgetV2/widget/index.tsx (1)
46-50: Nit: grouped helper imports improve readabilityWith three helpers coming from the same module, prefer a single import line:
-import { - getMetaFlexLayers, - isListFullyEmpty, - isTargetElementClickable, -} from "./helper"; +import { getMetaFlexLayers, isListFullyEmpty, isTargetElementClickable } from "./helper";
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/client/src/widgets/ListWidgetV2/MetaWidgetGenerator.ts(1 hunks)app/client/src/widgets/ListWidgetV2/widget/helper.ts(1 hunks)app/client/src/widgets/ListWidgetV2/widget/index.tsx(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/client/src/widgets/ListWidgetV2/widget/index.tsx (1)
app/client/src/widgets/ListWidgetV2/widget/helper.ts (1)
isListFullyEmpty(133-140)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: client-lint / client-lint
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
🔇 Additional comments (1)
app/client/src/widgets/ListWidgetV2/widget/index.tsx (1)
1471-1474: Empty-state logic now always returns “not empty”After injecting the placeholder
[{}],isListFullyEmptyinevitably returnsfalse, so the empty-state component will never render.
Double-check that this UX change is intended; if you still want to display “No data to display” you need a different predicate (e.g.originalDataWasEmptyflag).
Refactored the empty data handling logic in the ListWidget by removing the dependency on the isListFullyEmpty function. The check for empty data now solely relies on filtering the listData array, enhancing clarity and maintainability of the code. This change ensures that the widget correctly identifies and handles empty data scenarios without unnecessary complexity.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/15629332705. |
|
Deploy-Preview-URL: https://ce-40886.dp.appsmith.com |
Implemented a new test case to verify that when the List widget is provided with empty data, it correctly displays a "No data to display" message and ensures no errors are logged in the Appsmith console. This enhances the robustness of the List widget's empty state handling.
Updated the test case for the List widget to ensure that when provided with empty data, it correctly displays the "No data to display" message and verifies that no errors are logged in the Appsmith console. This change improves the robustness of the widget's handling of empty states.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicClientSideData_spec.js (1)
171-174: Selector is brittle; prefer a data-attribute over visible text
AssertElementVisibility(_.locators._visibleTextDiv("No data to display"))hard-codes UI copy. Any wording tweak will break the test while functionality remains intact. Expose an explicit data-cy (e.g.data-cy="list-empty-state") on the empty-state wrapper and assert against that:-_.agHelper.AssertElementVisibility( - _.locators._visibleTextDiv("No data to display"), -); +_.agHelper.AssertElementVisibility('[data-cy="list-empty-state"]');This follows Appsmith’s Cypress guidelines (data-* selectors, no raw strings) and keeps the spec resilient to copy changes.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicClientSideData_spec.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`app/client/cypress/**/**.*`: Review the following e2e test code written using the Cypress test library. Ensure that: - Follow best practices for Cypress code and e2e automation. ...
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicClientSideData_spec.js
🧠 Learnings (1)
📓 Common learnings
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#40886
File: app/client/src/widgets/ListWidgetV2/MetaWidgetGenerator.ts:1115-1134
Timestamp: 2025-06-13T07:49:21.394Z
Learning: For ListWidgetV2, an empty array supplied by users is internally converted to `[{}]` (a placeholder row) when `pageNo === 1`; this sentinel is never produced by user data, so a singleton empty object on page 1 can be safely treated as “empty list” in MetaWidgetGenerator.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
🔇 Additional comments (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicClientSideData_spec.js (1)
169-169:❌ Incorrect review comment
"[]"is set as a string, not a JS array
UpdatePropertyFieldValue("Items", "[]")injects literal text. If the property pane is in string mode, the list gets the characters[], not an empty array, and the test passes only because the binding still parses to an empty-ish value. To avoid an accidental regression, drive the control in JS mode:-_.propPane.UpdatePropertyFieldValue("Items", "[]"); +_.propPane.UpdatePropertyFieldValue("Items", "{{[]}}");This guarantees you’re testing the real empty-array path instead of relying on lenient coercion.
🏁 Script executed:
#!/bin/bash # Locate the definition and usages of UpdatePropertyFieldValue to verify its behavior rg "UpdatePropertyFieldValue" -n rg "propPane" -nLength of output: 73089
🏁 Script executed:
#!/bin/bash # Display the UpdatePropertyFieldValue method implementation for verification sed -n '380,430p' app/client/cypress/support/Pages/PropertyPane.tsLength of output: 1800
🏁 Script executed:
#!/bin/bash # Find definition of UpdateCodeInput in the agHelper rg "UpdateCodeInput" -n app/client/cypress/support/PagesLength of output: 747
🏁 Script executed:
#!/bin/bash # Show context around line 169 for Listv2_BasicClientSideData_spec.js sed -n '150,200p' app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicClientSideData_spec.jsLength of output: 899
Ignore wrapping the empty array in
{{…}}
The ListV2 “Items” field is a JSON/code editor by default, so calling_.propPane.UpdatePropertyFieldValue("Items", "[]");already injects an actual empty-array expression. No JS-toggle or mustache braces are needed here.
Likely an incorrect or invalid review comment.
…o rahulbarwal/fix-list-widget-issue
Updated the isListFullyEmpty function to accept an undefined listData parameter and return false if listData is not provided. Adjusted the ListWidget to use the updated function, ensuring it checks for the presence of data before attempting to push an empty object. This improves the robustness of the widget's empty state handling.
…o rahulbarwal/fix-list-widget-issue
…nerator ### 📝 Summary This commit introduces a new option `isEmptyListWidgetCase` to the `GeneratorOptions` interface and the `MetaWidgetGenerator` class, enhancing the handling of empty list scenarios in the ListWidget. ### 🎨 Changes Made - **MetaWidgetGenerator (`MetaWidgetGenerator.ts`)** - Added `isEmptyListWidgetCase` to `GeneratorOptions` and initialized it in the constructor. - Updated logic to utilize `isEmptyListWidgetCase` for determining empty data cases. - **ListWidget (`index.tsx`)** - Introduced a local variable `isEmptyListWidgetCase` to determine if the list is empty and adjusted the data binding accordingly. This change improves the robustness of the ListWidget's empty state handling, ensuring it correctly represents empty data scenarios without generating unnecessary meta widgets.
Description
Problem
Errors appeared in the console when the List widget in edit mode received an empty array as data.
Root cause
In edit mode, the List widget uses template widgets to create meta widgets. When the list becomes empty, meta widgets are not generated, but template widgets with
currentItembindings remain. SincecurrentItemrequires meta widgets for evaluation, this results in errors.Solution
This PR handles the generation of empty meta widgets when the List widget receives empty data, ensuring
currentItembindings don’t break. It introduces agenerateEmptyMetaWidgetsmethod inMetaWidgetGenerator, enhances the List widget to track and respond to empty data states, and resets the meta widget cache when transitioning between empty and non-empty states.Fixes #
Issue Numberor
Fixes #31925
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.List"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/15756809644
Commit: 58818e0
Cypress dashboard.
Tags:
@tag.ListSpec:
Thu, 19 Jun 2025 12:11:58 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Bug Fixes
New Features
Tests