feat: display label in table select cell #35124
Conversation
WalkthroughThe changes significantly enhance the select widget's functionality within table components, ensuring that selected options display their labels instead of their values. This improvement enriches the user experience, making the information presented more intuitive and meaningful. Users can better understand their selections, leading to clearer data representation within the application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TableWidget
participant SelectCell
participant OptionsArray
User->>TableWidget: Select option
TableWidget->>SelectCell: Update selected value
SelectCell->>OptionsArray: Retrieve label for selected value
OptionsArray-->>SelectCell: Return label
SelectCell->>TableWidget: Update cell content with label
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
github-actions
bot
added
Widgets Product
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10065798068. |
github-actions
bot
added
Bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts (4 hunks)
- app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx (3 hunks)
Additional context used
Path-based instructions (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts (1)
Pattern
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.
Additional comments not posted (3)
app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx (2)
192-197: Great use ofuseCallbackforgetCellLabelValue.This ensures that the function is memoized and only recalculated when
valueoroptionschange. This is efficient and aligns with React best practices.
199-199: Good job usinggetCellLabelValueforcellContent.This ensures that the cell displays the label of the selected option, enhancing user experience and consistency.
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts (1)
177-207: Well-written test case to validate label display.This test case effectively ensures that the label of the selected option is displayed as the cell content. It covers updating options, simulating selection, and asserting the correct display.
|
Deploy-Preview-URL: https://ce-35124.dp.appsmith.com |
|
@jacquesikot We have seen there is change in src with business logic. This logic can be tested with Jest unit tests or React testing library. Cypress is redundant here. |
There was a problem hiding this comment.
Why is this not part of #35096? I think we could've merged these.
app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx
Outdated
Show resolved
Hide resolved
app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx
Outdated
Show resolved
Hide resolved
jacquesikot
added
ok-to-test
@rahulbarwal After my call with Tom yesterday, the displayAs property is redundant, users can easily change what is displayed on the label from the selectOptions without needing that extra toggle. The more important property is the SortBy option |
github-actions
bot
added
Bug
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- app/client/src/ce/entities/FeatureFlag.ts (2 hunks)
- app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx
Additional comments not posted (2)
app/client/src/ce/entities/FeatureFlag.ts (2)
37-38: Good job adding the new feature flag!The new feature flag
release_table_cell_label_value_enabledis correctly added to theFEATURE_FLAGconstant. Please ensure it adheres to the naming conventions.
68-68: Nice work setting the default value!The new feature flag
release_table_cell_label_value_enabledis correctly added to theDEFAULT_FEATURE_FLAG_VALUEconstant with a default value offalse. This ensures the feature is disabled by default.
github-actions
bot
added
Bug
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts
github-actions
bot
added
Bug
jacquesikot
added
ok-to-test
## Description **Issue** In the table widget, the select column currently displays the value from the dropdown when a user makes a selection, rather than the label. This behaviour is inconsistent with the standalone select widget, which correctly renders the label upon selection. **Fix** This PR addresses the inconsistency by modifying the table widget's select column to display the label of the selected item, while maintaining the table cell value and ensuring alignment with the behaviour of the standalone select widget. **Tested Cases** Manual Test Cases for Table Select Widget Improvement 1. Default Value Display **Objective:** Ensure that a new table displays the correct default label key from the selectOptions in the cell. **Steps:** - Drop a new table widget. - Add sample data to the table. - Set a column (e.g., gender) to select type. - Verify that the table cell displays the value from the label key of the `selectOptions` by default. 2. Binding Check for Selected Row **Objective:** Ensure that the table binding for the selected row reflects the correct value key from the selectOptions. **Steps:** - Drop a new table widget. - Add sample data to the table. - Set a column (e.g., gender) to select type. - Select a row and verify that `Table1.selectedRow.gender` matches the value key from the `selectOptions`. 3. Updating Table Cell Content **Objective:** Verify that updating the table cell content via the dropdown updates the cell with the correct label key property. **Steps:** - Drop a new table widget. - Add sample data to the table. - Set a column (e.g., gender) to select type. - Change the content of the table cell using the dropdown. - Confirm that the cell content updates accordingly with the value key property. 4. Add New Row Functionality **Objective:** Ensure that adding a new row works as expected with the enhanced functionality. **Steps:** - Drop a new table widget. - Add sample data to the table. - Set a column (e.g., gender) to select type. - Add a new row to the table. - Verify that the new row uses the label key property from the `selectOptions` in the table display. 5. Deployed Mode Verification **Objective:** Verify that the display functionality works correctly in deployed mode. **Steps:** - Drop a new table widget. - Add sample data to the table. - Set a column (e.g., gender) to select type. - Change the displayAs property to both value and label. - Deploy the table. - Verify that the table displays correctly in both value and label modes in the deployed environment. Fixes appsmithorg#26188 ## Automation /ok-to-test tags="@tag.Table, @tag.Binding, @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/10080592979> > Commit: 78b268d > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10080592979&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Table, @tag.Binding, @tag.Sanity` > Spec: > <hr>Wed, 24 Jul 2024 17:17:06 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** - Improved display of selected options in table widgets by showing the corresponding label instead of the raw value. - Introduced a feature flag to toggle the table cell label value functionality. - **Bug Fixes** - Enhanced test cases to ensure accurate validation of select options behavior within the table widget. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/10244888836. |
## 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 -->
Description
Issue
In the table widget, the select column currently displays the value from the dropdown when a user makes a selection, rather than the label. This behaviour is inconsistent with the standalone select widget, which correctly renders the label upon selection.
Fix
This PR addresses the inconsistency by modifying the table widget's select column to display the label of the selected item, while maintaining the table cell value and ensuring alignment with the behaviour of the standalone select widget.
Tested Cases
Manual Test Cases for Table Select Widget Improvement
Objective: Ensure that a new table displays the correct default label key from the selectOptions in the cell.
Steps:
selectOptionsby default.Objective: Ensure that the table binding for the selected row reflects the correct value key from the selectOptions.
Steps:
Table1.selectedRow.gendermatches the value key from theselectOptions.Objective: Verify that updating the table cell content via the dropdown updates the cell with the correct label key property.
Steps:
Objective: Ensure that adding a new row works as expected with the enhanced functionality.
Steps:
selectOptionsin the table display.Objective: Verify that the display functionality works correctly in deployed mode.
Steps:
Fixes #26188
Automation
/ok-to-test tags="@tag.Table, @tag.Binding, @tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10080592979
Commit: 78b268d
Cypress dashboard.
Tags:
@tag.Table, @tag.Binding, @tag.SanitySpec:
Wed, 24 Jul 2024 17:17:06 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes