Skip to content

Commit

Permalink
fix: Ensure Select Column Displays Data When Options Are Not Set (#35817
Browse files Browse the repository at this point in the history
)

## Description
**Problem**
When using a table widget, changing a column to the "Select" type causes
the data in that column to disappear unless the options property is
explicitly set. This creates a poor user experience, as it appears that
the data has been lost or the widget is malfunctioning.

**Solution**
We have implemented a fallback mechanism to ensure a smoother user
experience. If no options are set in the property pane, the label inside
the select cell will now default to displaying the existing value. This
ensures that the data remains visible, even in the absence of predefined
options.


Fixes #35807

## Automation

/ok-to-test tags="@tag.Widget, @tag.Select, @tag.Binding, @tag.Table,
@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/10513374411>
> Commit: 1e89468
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10513374411&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Widget, @tag.Select, @tag.Binding, @tag.Table,
@tag.Sanity`
> Spec:
> <hr>Thu, 22 Aug 2024 19:43:53 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**
- Enhanced the select column in the table widget to return a default
value when no options are available.

- **Bug Fixes**
- Improved robustness of the select cell component to prevent errors
when the options array is empty.

- **Tests**
- Updated and reorganized test cases for the select column to improve
clarity and specificity.
- Introduced a new test case to validate default value behavior when no
options are provided.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
jacquesikot authored Aug 23, 2024
1 parent 0015466 commit 28ac53b
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,13 @@ describe(
cy.get(".t--property-pane-section-collapse-events").should("exist");
});

it("2. should check that options given in the property pane is appearing on the table", () => {
it("2. should check that select column returns value if no option is provided", () => {
cy.readTableV2data(0, 0).then((val) => {
expect(val).to.equal("#1");
});
});

it("3. should check that options given in the property pane is appearing on the table", () => {
cy.get(".t--property-control-options").should("exist");
cy.updateCodeInput(
".t--property-control-options",
Expand Down Expand Up @@ -74,7 +80,7 @@ describe(
cy.get(".menu-item-active.has-focus").should("contain", "#1");
});

it("3. should check that placeholder property is working", () => {
it("4. should check that placeholder property is working", () => {
cy.updateCodeInput(
".t--property-control-options",
`
Expand Down Expand Up @@ -110,7 +116,7 @@ describe(
).should("contain", "choose an item");
});

it("4. should check that filterable property is working", () => {
it("5. should check that filterable property is working", () => {
cy.updateCodeInput(
".t--property-control-options",
`
Expand Down Expand Up @@ -155,7 +161,7 @@ describe(
cy.get(".t--canvas-artboard").click({ force: true });
});

it("5. should check that on option select is working", () => {
it("6. should check that on option select is working", () => {
featureFlagIntercept({ release_table_cell_label_value_enabled: true });
cy.openPropertyPane("tablewidgetv2");
cy.editColumn("step");
Expand Down Expand Up @@ -197,7 +203,7 @@ describe(
cy.discardTableRow(4, 0);
});

it("6. should check that currentRow is accessible in the select options", () => {
it("7. should check that currentRow is accessible in the select options", () => {
cy.updateCodeInput(
".t--property-control-options",
`
Expand All @@ -222,7 +228,7 @@ describe(
cy.get(".menu-item-text").contains("#1").should("not.exist");
});

it("7. should check that 'same select option in new row' property is working", () => {
it("8. should check that 'same select option in new row' property is working", () => {
_.propPane.NavigateBackToPropertyPane();

const checkSameOptionsInNewRowWhileEditing = () => {
Expand Down Expand Up @@ -288,7 +294,7 @@ describe(
checkSameOptionsWhileAddingNewRow();
});

it("8. should check that 'new row select options' is working", () => {
it("9. should check that 'new row select options' is working", () => {
const checkNewRowOptions = () => {
// New row select options should be visible when "Same options in new row" is turned off
_.propPane.TogglePropertyState("Same options in new row", "Off");
Expand Down Expand Up @@ -353,7 +359,7 @@ describe(
checkNoOptionState();
});

it("9. should check that server side filering is working", () => {
it("10. should check that server side filering is working", () => {
_.dataSources.CreateDataSource("Postgres");
_.dataSources.CreateQueryAfterDSSaved(
"SELECT * FROM public.astronauts {{this.params.filterText ? `WHERE name LIKE '%${this.params.filterText}%'` : ''}} LIMIT 10;",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ export const SelectCell = (props: SelectProps) => {

const cellLabelValue = useMemo(() => {
if (releaseTableSelectCellLabelValue) {
if (!options.length) return value;
const selectedOption = options.find(
(option) => option[TableSelectColumnOptionKeys.VALUE] === value,
);
Expand Down

0 comments on commit 28ac53b

Please sign in to comment.