Skip to content

fix: Multiselect widget selected values inside list widget #41214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: release
Choose a base branch
from
+63 −10
Open

fix: Multiselect widget selected values inside list widget #41214

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a55fd9b
fix: Multiselect widget selected values inside list widget
saharAdem Sep 10, 2025
633558c
Resolve comments
saharAdem Sep 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions app/client/cypress/e2e/Regression/ClientSide/BugTests/MultiselectWidget_Bug 41210 _Spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { locators } from "../../../../support/Objects/ObjectsCore";
import * as _ from "../../../../support/Objects/ObjectsCore";

const widgetSelector = (name: string) => `[data-widgetname-cy="${name}"]`;

describe("Bug 41210: MultiSelectWidgetV2 inside ListWidget - selected values and labels persist per item", function () {
before(() => {
_.agHelper.AddDsl("Listv2/emptyList");
});

it("should persist selected values for each list item and initialize with default values on first render", function () {
cy.dragAndDropToWidget("multiselectwidgetv2", "containerwidget", {
x: 250,
y: 50,
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid using coordinate-based drag and drop

Using fixed x,y coordinates (250, 50) for drag and drop operations is brittle and may fail on different screen sizes or when the UI changes. Consider using a more robust approach.

#!/bin/bash
# Check if there are helper methods available for drag and drop operations without coordinates
rg -n --type=ts "dragAndDropToWidget.*widgetSelector" -A 3 -B 3
🤖 Prompt for AI Agents 
In app/client/cypress/e2e/Regression/ClientSide/BugTests/MultiselectWidget_Bug
41210 _Spec.ts around lines 12 to 15, the test uses fixed x,y coordinates for
cy.dragAndDropToWidget which is brittle; update the call to use a selector-based
target (or the helper's widgetSelector option) so the drop is done relative to
the target element instead of absolute coordinates — either pass the destination
widget selector to cy.dragAndDropToWidget (remove the x/y object) or compute the
target's center via cy.get('<target-selector>').then(el => { use its bounding
box to perform the drag/drop }) so the test works across screen sizes and layout
changes.

_.propPane.UpdatePropertyFieldValue("Default selected values", '["GREEN"]');
_.agHelper.GetNClick(locators._enterPreviewMode);
_.agHelper.SelectFromMultiSelect(["Red"]);

const listContainer = `${widgetSelector("List1")} [type="CONTAINER_WIDGET"]`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid using CSS attribute selectors

The selector [type="CONTAINER_WIDGET"] violates the coding guidelines which specify avoiding attribute selectors. Use data-* attributes instead.

-    const listContainer = `${widgetSelector("List1")} [type="CONTAINER_WIDGET"]`;
+    const listContainer = `${widgetSelector("List1")} [data-testid="list-container"]`;

Note: Ensure the corresponding data-testid attribute is added to the container widget in the application code.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const listContainer = `${widgetSelector("List1")} [type="CONTAINER_WIDGET"]`;
const listContainer = `${widgetSelector("List1")} [data-testid="list-container"]`;
🤖 Prompt for AI Agents 
In app/client/cypress/e2e/Regression/ClientSide/BugTests/MultiselectWidget_Bug
41210 _Spec.ts around line 20, the selector uses a CSS attribute selector
`[type="CONTAINER_WIDGET"]` which violates guidelines; replace it with a
data-testid selector and update the application to add that data-testid to the
container widget. Change the test to target the container via its data-testid
(e.g., `${widgetSelector("List1")} [data-testid="container-widget-List1"]` or a
stable name you add in the app) and add the corresponding data-testid attribute
to the container widget in the app code.


cy.get(listContainer).eq(1).click();
cy.get(listContainer).eq(0).click();
cy.get(
`${widgetSelector("MultiSelect1")} .rc-select-selection-item`,
).should("contain.text", "Red");
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Test lacks comprehensive assertions

The test only verifies that "Red" is selected but doesn't verify:

  1. That the default value "GREEN" was initially selected
  2. That other list items maintain their own selections
  3. That the selection persists after multiple navigations
   it("should persist selected values for each list item and initialize with default values on first render", function () {
     cy.dragAndDropToWidget("multiselectwidgetv2", "containerwidget", {
       x: 250,
       y: 50,
     });
     _.propPane.UpdatePropertyFieldValue("Default selected values", '["GREEN"]');
     _.agHelper.GetNClick(locators._enterPreviewMode);
+    
+    // Verify default value is selected
+    cy.get(
+      `${widgetSelector("MultiSelect1")} .rc-select-selection-item`,
+    ).should("contain.text", "Green");
+    
     _.agHelper.SelectFromMultiSelect(["Red"]);
 
     const listContainer = `${widgetSelector("List1")} [type="CONTAINER_WIDGET"]`;
 
+    // Navigate to second item and verify it has the default value
     cy.get(listContainer).eq(1).click();
+    cy.get(
+      `${widgetSelector("MultiSelect1")} .rc-select-selection-item`,
+    ).should("contain.text", "Green");
+    
+    // Return to first item and verify Red persists
     cy.get(listContainer).eq(0).click();
     cy.get(
       `${widgetSelector("MultiSelect1")} .rc-select-selection-item`,
     ).should("contain.text", "Red");
+    
+    // Navigate again to ensure persistence
+    cy.get(listContainer).eq(1).click();
+    cy.get(listContainer).eq(0).click();
+    cy.get(
+      `${widgetSelector("MultiSelect1")} .rc-select-selection-item`,
+    ).should("contain.text", "Red");
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it("should persist selected values for each list item and initialize with default values on first render", function () {
cy.dragAndDropToWidget("multiselectwidgetv2", "containerwidget", {
x: 250,
y: 50,
});
_.propPane.UpdatePropertyFieldValue("Default selected values", '["GREEN"]');
_.agHelper.GetNClick(locators._enterPreviewMode);
_.agHelper.SelectFromMultiSelect(["Red"]);
const listContainer = `${widgetSelector("List1")} [type="CONTAINER_WIDGET"]`;
cy.get(listContainer).eq(1).click();
cy.get(listContainer).eq(0).click();
cy.get(
`${widgetSelector("MultiSelect1")} .rc-select-selection-item`,
).should("contain.text", "Red");
});
it("should persist selected values for each list item and initialize with default values on first render", function () {
cy.dragAndDropToWidget("multiselectwidgetv2", "containerwidget", {
x: 250,
y: 50,
});
_.propPane.UpdatePropertyFieldValue("Default selected values", '["GREEN"]');
_.agHelper.GetNClick(locators._enterPreviewMode);
// Verify default value is selected
cy.get(
`${widgetSelector("MultiSelect1")} .rc-select-selection-item`,
).should("contain.text", "Green");
_.agHelper.SelectFromMultiSelect(["Red"]);
const listContainer = `${widgetSelector("List1")} [type="CONTAINER_WIDGET"]`;
// Navigate to second item and verify it has the default value
cy.get(listContainer).eq(1).click();
cy.get(
`${widgetSelector("MultiSelect1")} .rc-select-selection-item`,
).should("contain.text", "Green");
// Return to first item and verify Red persists
cy.get(listContainer).eq(0).click();
cy.get(
`${widgetSelector("MultiSelect1")} .rc-select-selection-item`,
).should("contain.text", "Red");
// Navigate again to ensure persistence
cy.get(listContainer).eq(1).click();
cy.get(listContainer).eq(0).click();
cy.get(
`${widgetSelector("MultiSelect1")} .rc-select-selection-item`,
).should("contain.text", "Red");
});
🤖 Prompt for AI Agents 
app/client/cypress/e2e/Regression/ClientSide/BugTests/MultiselectWidget_Bug
41210 _Spec.ts lines 11-27: the test only asserts that "Red" is selected but
misses verifying the initial default "GREEN", per-item selection independence,
and persistence after navigation; update the test to first assert that on first
render MultiSelect1 contains "GREEN" (the default), then select a different
option for a second list item and assert that each list item shows its own
selected value (e.g., item0 shows "GREEN", item1 shows "Red"), then navigate
between list items (click other indices) multiple times and re-check the
selections to confirm persistence across navigations; use the same widget
selectors already present (widgetSelector("MultiSelect1") and listContainer
indices) and add assertions after each navigation step to validate the expected
text values.

});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix the filename to follow naming conventions

The filename contains a space between "Bug" and "41210" which violates standard naming conventions. Rename the file to use underscores or camelCase consistently.

-MultiselectWidget_Bug 41210 _Spec.ts
+MultiselectWidget_Bug41210_Spec.ts

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In app/client/cypress/e2e/Regression/ClientSide/BugTests/MultiselectWidget_Bug
41210 _Spec.ts lines 1-28: the filename contains a space ("Bug 41210") which
violates repo naming conventions; rename the file to remove spaces (e.g.,
MultiselectWidget_Bug_41210_Spec.ts or multiselectWidgetBug41210Spec.ts) using
git mv, update any CI/test references or import paths that point to the old
filename, and ensure the test runner configuration or any documentation is
updated accordingly so the test still runs under the new name.

27 changes: 25 additions & 2 deletions app/client/src/widgets/MultiSelectWidgetV2/widget/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -836,12 +836,29 @@ class MultiSelectWidget extends BaseWidget<
}

getWidgetView() {
const {
currentIndex,
defaultOptionValue = [],
selectedValuesByItem = {},
updateWidgetMetaProperty,
} = this.props;

const itemId = String(currentIndex);
let values = selectedValuesByItem[itemId] || defaultOptionValue;

if (!selectedValuesByItem[itemId] && defaultOptionValue) {
values = defaultOptionValue as string[];
updateWidgetMetaProperty("selectedValuesByItem", {
...selectedValuesByItem,
[itemId]: values,
});
}

const options = isArray(this.props.options) ? this.props.options : [];
const minDropDownWidth =
(MinimumPopupWidthInPercentage / 100) *
(this.props.mainCanvasWidth ?? layoutConfigurations.MOBILE.maxWidth);
const { componentHeight, componentWidth } = this.props;
const values = this.mergeLabelAndValue();
const isInvalid =
"isValid" in this.props && !this.props.isValid && !!this.props.isDirty;

Expand Down Expand Up @@ -887,7 +904,13 @@ class MultiSelectWidget extends BaseWidget<
}

onOptionChange = (value: DraftValueType) => {
this.props.updateWidgetMetaProperty("selectedOptions", value, {
const itemId = this.props.currentIndex;
const updatedValue = {
...(this.props.selectedValuesByItem || {}),
[itemId]: value,
};

this.props.updateWidgetMetaProperty("selectedValuesByItem", updatedValue, {
triggerPropertyName: "onOptionChange",
dynamicString: this.props.onOptionChange,
event: {
Expand Down