fix: select column to display cell value when options are mismatched #38387
Conversation
jacquesikot
added
the
Widgets & Accelerators Pod
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
Bug
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12512977970. |
![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
🧹 Nitpick comments (1)
app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx (1)
207-207: Consider null/undefined values when no matching option is found.Returning
valueensures that valid but unlisted entries are displayed. However, ensure thatvaluecan never benullorundefined; otherwise, it might render incorrectly. A quick null/undefined check or fallback for such cases could improve robustness.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/widgets/TableWidgetV2/component/cellComponents/SelectCell.tsx(1 hunks)
|
Deploy-Preview-URL: https://ce-38387.dp.appsmith.com |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts (1)
410-410: Correct the typographical error
“filering” should be spelled “filtering.”-it("12. should check that server side filering is working", () => { +it("12. should check that server side filtering is working", () => {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts(2 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 (2)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts (2)
339-343: All good for mismatch scenario
This test correctly verifies that the table cell displays its existing value even if it doesn't match any of the provided options, which aligns perfectly with the PR's objective.
345-345: Comprehensive testing of new row select options
Good job handling multiple sub-scenarios to ensure proper behavior of new row select options.
Description
Problem
When a table cell is configured with the Select column type, the cell value is not displayed if it does not match any of the label-value pairs in the select options. This results in a broken user experience where it appears as if the cell has no value.
Expected Behaviour
The table cell should always display its value if it exists, regardless of whether it matches the provided select options.
Solution
To address this issue, the default behaviour for the select options has been updated to prioritise the cell's value instead of falling back to an empty string. This ensures that cell values are always visible to the user, even if they are not part of the predefined select options.
Fixes #35807
Automation
/ok-to-test tags=""
🔍 Cypress test results
Warning
Tests have not run on the HEAD d39d7f3 yet
Fri, 27 Dec 2024 07:22:19 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit