Skip to content

Commit

Permalink
feat: Allow filtering of table select column label (#36755)
Browse files Browse the repository at this point in the history
## Description

**Problem**
When filtering a table with a select column type, users expect to filter
by the visible label values shown in each cell. Currently, however,
filtering is applied to the underlying option values rather than the
displayed labels, leading to unexpected filter results for end-users.

**Root Cause**
In a previous update ([PR
#35124](#35124)), the table
cell display for select columns was changed to show labels instead of
values. However, the filtering logic was not updated accordingly, so the
table still filtered on the original option values, creating a mismatch
between displayed and filtered content.

**Solution**
This PR modifies the displayedRow property within the table widget to
use the label property instead of the value key when filtering or
searching select column data. This ensures that table filtering and
searching now align with the visible label values in the select columns,
providing a more intuitive user experience.


Fixes #36635 

## Automation

/ok-to-test tags="@tag.Sanity, @tag.Table, @tag.Select, @tag.Binding"

### 🔍 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/11234735437>
> Commit: fd6c179
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11234735437&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Sanity, @tag.Table, @tag.Select, @tag.Binding`
> Spec:
> <hr>Tue, 08 Oct 2024 12:48:01 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

- **New Features**
- Introduced a new test case to verify filtering functionality for the
"role" column in the Table Widget.
- Enhanced filtering mechanism to support multiple label values for
select columns.

- **Bug Fixes**
	- Removed outdated search functionality to streamline user experience.

- **Refactor**
	- Restructured existing test cases for improved clarity and flow.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
jacquesikot authored Oct 9, 2024
1 parent da8f726 commit d233d7e
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,17 @@ describe(
expect(afterSearch).to.eq("Software Engineer");
});
table.RemoveSearchTextNVerify("1", "v2");
});

// Search for a value in the table
table.SearchTable("20");
table.ReadTableRowColumnData(0, 2, "v2").then((afterSearch) => {
expect(afterSearch).to.eq("Product Manager");
it("12. Verify table filter for select column type", function () {
featureFlagIntercept({ release_table_cell_label_value_enabled: true });
table.OpenNFilterTable("role", "is exactly", "Product Manager");
table.ReadTableRowColumnData(0, 2, "v2").then(($cellData) => {
expect($cellData).to.eq("Product Manager");
});
table.ReadTableRowColumnData(1, 2, "v2").then(($cellData) => {
expect($cellData).to.eq("Product Manager");
});
table.RemoveSearchTextNVerify("1", "v2");
});
},
);
16 changes: 9 additions & 7 deletions app/client/src/widgets/TableWidgetV2/widget/derived.js
Original file line number Diff line number Diff line change
Expand Up @@ -682,9 +682,9 @@ export default {

/*
* For select columns with label and values, we need to include the label value
* in the search
* in the search and filter data
*/
let labelValueForSelectCell = "";
let labelValuesForSelectCell = {};
/*
* Initialize an array to store keys for columns that have the 'select' column type
* and contain selectOptions.
Expand Down Expand Up @@ -716,13 +716,15 @@ export default {
primaryColumns[key].selectOptions,
);

let selectOptions;
let selectOptions = {};

/*
* If selectOptions is an array, check if it contains nested arrays.
* This is to handle situations where selectOptons is a javascript object and computes as a nested array.
*/
if (isSelectOptionsAnArray) {
const selectOptionKey = primaryColumns[key].alias;

if (_.some(primaryColumns[key].selectOptions, _.isArray)) {
/* Handle the case where selectOptions contains nested arrays - selectOptions is javascript */
selectOptions =
Expand All @@ -732,7 +734,7 @@ export default {
});

if (option) {
labelValueForSelectCell = option.label;
labelValuesForSelectCell[selectOptionKey] = option.label;
}
} else {
/* Handle the case where selectOptions is a flat array - selectOptions is plain JSON */
Expand All @@ -742,7 +744,7 @@ export default {
);

if (option) {
labelValueForSelectCell = option.label;
labelValuesForSelectCell[selectOptionKey] = option.label;
}
}
} else {
Expand All @@ -753,15 +755,15 @@ export default {
);

if (option) {
labelValueForSelectCell = option.label;
labelValuesForSelectCell[selectOptionKey] = option.label;
}
}
});
}

const displayedRow = {
...row,
labelValueForSelectCell,
...labelValuesForSelectCell,
...columnWithDisplayText.reduce((acc, column) => {
let displayText;

Expand Down

0 comments on commit d233d7e

Please sign in to comment.