feat: Implements HTML as a column type in table widget. #37997
Conversation
- Introduced a new column type "HTML" in the TableWidgetV2 constants. - Created a new HTMLCell component to render HTML content within table cells. - Updated the TableWidgetV2 to support rendering of HTML cells. - Enhanced the property configuration to include the new HTML column type. This update improves the flexibility of the TableWidgetV2 by allowing HTML content to be displayed directly in table cells.
- Introduced new test files for AutoTooltipComponent, BasicCell, HTMLCell, and PlainTextCell to enhance test coverage. - Implemented various test cases to validate rendering, tooltip functionality, and HTML content handling. - Ensured proper behavior for different column types and edge cases, including truncated text and null values. These additions improve the reliability and maintainability of the TableWidgetV2 components by ensuring they function as expected across various scenarios.
- Introduced a new column type `HTML` in the `ColumnTypes` enum to support HTML content in table widgets. - Updated the CSV transformation logic to conditionally quote values based on the new `HTML` column type, enhancing data handling in CSV exports. This change improves the flexibility of the table widget and ensures proper formatting of HTML content during CSV export.
- Introduced a new column type `HTML` in the `ColumnTypes` enum to allow HTML content in table widgets. - Updated filtering and sorting logic to handle HTML columns, including a new utility function to extract text from HTML. - Created a new constants file for table widget filter operators to streamline operator management. - Enhanced unit tests to validate functionality for HTML columns, including search, filter, and sort operations. This update improves the flexibility and usability of the TableWidgetV2 by enabling the display and manipulation of HTML content directly within table cells.
… types more effectively.
- Updated test cases to validate HTML content rendering, filtering, and searching within the TableWidgetV2. - Modified status values to include inline styles for better visual representation. - Added tests to ensure tags are not filterable or searchable in HTML content. - Implemented complex filtering scenarios with multiple conditions and HTML entities. This update improves the robustness of the TableWidgetV2 by ensuring accurate handling of HTML content in various scenarios.
github-actions
bot
added
Enhancement
WalkthroughThis pull request introduces extensive enhancements to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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
Enhancement
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12193187943. |
rahulbarwal
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (10)
app/client/src/widgets/TableWidgetV2/widget/derived.js (6)
Line range hint
1831-1831: Avoid using thedeleteoperator for better performanceDeleting object properties can impact performance in JavaScript. Consider setting
input.searchTexttoundefinedinstead.Apply this diff to refactor:
-delete input.searchText; +input.searchText = undefined;
Line range hint
1860-1860: Avoid using thedeleteoperator for better performanceSimilarly, consider setting
input.filterstoundefinedinstead of deleting it.Apply this diff:
-delete input.filters; +input.filters = undefined;
Line range hint
1939-1939: Avoid using thedeleteoperator for better performanceConsider replacing the
deleteoperation with an assignment toundefinedforinput.filters.-delete input.filters; +input.filters = undefined;
Line range hint
1954-1954: Avoid using thedeleteoperator for better performanceSet
input.searchTexttoundefinedinstead of deleting the property.-delete input.searchText; +input.searchText = undefined;
Line range hint
2038-2038: Avoid using thedeleteoperator for better performanceReplace the
deletestatement with an assignment toundefinedforinput.filters.-delete input.filters; +input.filters = undefined;
Line range hint
2089-2090: Avoid using thedeleteoperator for better performanceFor both
complexHTMLInput.searchTextandcomplexHTMLInput.filters, set them toundefinedinstead of deleting.Apply this diff:
-delete complexHTMLInput.searchText; -delete complexHTMLInput.filters; +complexHTMLInput.searchText = undefined; +complexHTMLInput.filters = undefined;app/client/src/widgets/TableWidgetV2/component/cellComponents/HTMLCell.tsx (2)
8-69: Add missing h4 heading styles for consistencyThe HTMLContainer component includes styles for h1-h3 and h5-h6, but h4 is missing despite being included in the general heading styles.
Add the missing h4 styles:
h3 { font-size: 1.17em; margin: 0.83em 0; } + h4 { + font-size: 1em; + margin: 1.33em 0; + } h5 {
111-115: Consider adding sanitization optionsWhile Interweave provides basic sanitization, consider exposing sanitization options through props for more granular control.
app/client/src/widgets/TableWidgetV2/component/cellComponents/__tests__/HTMLCell.test.tsx (2)
29-40: Add return type annotation for renderComponentThe utility function should have an explicit return type for better type safety.
const renderComponent = ( props: Partial<HTMLCellProps> = {}, store = unitTestBaseMockStore, - ) => { + ): ReturnType<typeof render> => {
105-138: Enhance security test coverageWhile the current security tests are good, consider adding tests for:
- iframe elements
- javascript: URLs
- data: URLs
- event handlers other than onclick
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (11)
app/client/src/widgets/TableWidgetV2/component/cellComponents/HTMLCell.tsx(1 hunks)app/client/src/widgets/TableWidgetV2/component/cellComponents/__tests__/HTMLCell.test.tsx(1 hunks)app/client/src/widgets/TableWidgetV2/component/header/actions/Utilities.test.ts(1 hunks)app/client/src/widgets/TableWidgetV2/component/header/actions/Utilities.ts(2 hunks)app/client/src/widgets/TableWidgetV2/component/header/actions/filter/CascadeFields.tsx(3 hunks)app/client/src/widgets/TableWidgetV2/component/header/actions/filter/Constants.ts(1 hunks)app/client/src/widgets/TableWidgetV2/constants.ts(3 hunks)app/client/src/widgets/TableWidgetV2/widget/derived.js(5 hunks)app/client/src/widgets/TableWidgetV2/widget/derived.test.js(1 hunks)app/client/src/widgets/TableWidgetV2/widget/index.tsx(2 hunks)app/client/src/widgets/TableWidgetV2/widget/propertyConfig/PanelConfig/Data.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/src/widgets/TableWidgetV2/widget/derived.test.js
[error] 1831-1831: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1860-1860: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1939-1939: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1954-1954: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 2038-2038: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 2089-2090: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (15)
app/client/src/widgets/TableWidgetV2/widget/derived.js (5)
284-291: Well-implemented getTextFromHTML function
The getTextFromHTML utility correctly extracts text content from HTML strings, ensuring that HTML tags do not interfere with search, filter, and sort functionalities.
515-519: Correct sorting for HTML columns
Using getTextFromHTML in the sorting logic ensures that HTML columns are sorted based on their text content, providing accurate sorting behavior.
716-719: Efficient identification of HTML columns
Filtering columns by columnType === "html" effectively identifies HTML columns for further processing.
818-823: Proper exclusion of HTML tags from search
By extracting the text content from HTML columns using getTextFromHTML, you prevent HTML tags and inline styles from affecting search results, enhancing search accuracy.
870-880: Accurate filtering for HTML columns
Applying getTextFromHTML to both the original and displayed column values ensures that filtering conditions are evaluated based on text content, improving filter reliability for HTML columns.
app/client/src/widgets/TableWidgetV2/widget/derived.test.js (1)
1649-2172: Comprehensive tests for HTML column functionality
The added test suite thoroughly covers searching, filtering, and sorting operations for HTML columns, enhancing test coverage and ensuring reliable functionality.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1831-1831: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1860-1860: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1939-1939: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1954-1954: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 2038-2038: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 2089-2090: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
app/client/src/widgets/TableWidgetV2/component/header/actions/Utilities.ts (1)
39-43: Proper CSV quoting for HTML columns
Including ColumnTypes.HTML in the shouldQuote condition ensures that HTML content is correctly quoted, preserving the integrity of HTML data in the exported CSV.
app/client/src/widgets/TableWidgetV2/component/header/actions/Utilities.test.ts (1)
93-116: Adequate test for CSV transformation with HTML content
The new test case effectively verifies that transformTableDataIntoCsv correctly handles HTML content, ensuring accurate CSV exports.
app/client/src/widgets/TableWidgetV2/component/header/actions/filter/Constants.ts (1)
1-45: Organized filter operators definition
Defining TABLE_WIDGET_FILTER_OPERATORS enhances code maintainability by centralizing filter operator configurations.
app/client/src/widgets/TableWidgetV2/widget/propertyConfig/PanelConfig/Data.ts (2)
45-48: LGTM: HTML column type configuration added correctly
The HTML column type is properly integrated into the column type dropdown options.
180-180: LGTM: HTML column type added to computedValue property
HTML column type is correctly added to the list of column types that hide the computedValue property.
app/client/src/widgets/TableWidgetV2/component/header/actions/filter/CascadeFields.tsx (2)
176-180: LGTM: HTML column filter operators are appropriate and secure
The HTML column type correctly implements essential text search operators (contains, does not contain) and existence checks (empty, not empty). This is appropriate for HTML content as it allows searching within the text content while maintaining security.
199-199: LGTM: HTML type mapping added
HTML column type is correctly mapped in the columnTypeNameMap.
app/client/src/widgets/TableWidgetV2/widget/index.tsx (2)
142-142: LGTM: HTMLCell component imported
Clean import of the HTMLCell component.
2521-2538: LGTM: HTML cell rendering implementation is complete
The HTML cell implementation:
- Correctly handles all necessary styling props
- Manages cell visibility and disabled states
- Follows the established pattern for cell rendering
✅ Verification successful
HTML cell implementation is secure and well-tested
The implementation is verified to be robust and secure:
- Uses Interweave for HTML sanitization
- Comprehensive test coverage for XSS prevention
- Proper handling of null/undefined values
- Well-styled HTML elements with proper hierarchy
- Link handling with security considerations (newWindow prop)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify HTMLCell component implementation
# Check if HTMLCell has proper sanitization
ast-grep --pattern 'import { Interweave } from "interweave"'
# Check if HTMLCell has proper tests
fd HTMLCell.test.tsx
Length of output: 175
Script:
#!/bin/bash
# Check HTMLCell implementation for proper sanitization
ast-grep --pattern 'function HTMLCell' -A 20
# Check content of HTMLCell test file
cat app/client/src/widgets/TableWidgetV2/component/cellComponents/__tests__/HTMLCell.test.tsx
Length of output: 4559
Script:
#!/bin/bash
# Check HTMLCell implementation for sanitization
cat app/client/src/widgets/TableWidgetV2/component/cellComponents/HTMLCell.tsx
Length of output: 2743
app/client/src/widgets/TableWidgetV2/component/cellComponents/HTMLCell.tsx
Outdated
Show resolved
Hide resolved
| @@ -146,6 +146,7 @@ export enum ColumnTypes { | |||
| CHECKBOX = "checkbox", | |||
| SWITCH = "switch", | |||
| CURRENCY = "currency", | |||
| HTML = "html", | |||
There was a problem hiding this comment.
💡 Codebase verification
HTML column type integration is incomplete
The HTML column type has been added to the enums but lacks proper implementation in switch statements handling column types. This needs to be addressed in:
TableWidget/component/TableUtilities.tsxTableWidget/component/CommonUtilities.tsTableWidgetV2/component/cellComponents/PlainTextCell.tsxTableWidgetV2/component/cellComponents/BasicCell.tsxTableWidgetV2/component/cellComponents/AutoToolTipComponent.tsx
Only TableWidgetV2/widget/index.tsx has the HTML case implementation with HTMLCell component.
🔗 Analysis chain
Verify HTML column type integration
The HTML column type has been added to all necessary enums and arrays. Let's verify its usage across the codebase.
Also applies to: 162-162, 170-170, 181-181
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any existing usage of HTML column type
rg -l "ColumnTypes\.HTML" --type ts --type tsx
# Check for column type handling in transformers
ast-grep --pattern 'switch ($expr) {
$$$
case ColumnTypes.HTML:
$$$
}'
Length of output: 157
Script:
#!/bin/bash
# Check for any existing usage of HTML column type, fixing the file type flag
rg -l "ColumnTypes\.HTML" -t ts
# Check for column type handling in transformers and processors
rg -A 5 "case.*HTML" -t ts
# Look for potential HTML column type implementations
rg -A 5 "HTML.*column" -t ts
# Search for switch statements handling column types
rg -A 10 "switch.*columnType" -t ts
Length of output: 8890
- Removed the Constants.ts file and integrated filter operators directly into CascadeFields.tsx. streamlining the filtering process for different column types.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12193306121. |
|
Deploy-Preview-URL: https://ce-37997.dp.appsmith.com |
…null or undefined HTML inputs, improving robustness.
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12236478046. |
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
|
Deploy-Preview-URL: https://ce-37997.dp.appsmith.com |
…operties, specifically for HTML content.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12250251609. |
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
|
Deploy-Preview-URL: https://ce-37997.dp.appsmith.com |
| case "string": | ||
| return dateFormatOptions.some(({ value: format }) => | ||
| case "string": { | ||
| const isHTML = /<[^>]*>/.test(columnValue as string); |
There was a problem hiding this comment.
This method doesn't seem exhaustive enough, there's a high chance of false positives with this, can we use something like this?
function detectHTML(input: string): boolean {
const div = document.createElement("div");
div.innerHTML = input;
return div.innerHTML !== input; // If innerHTML modifies input, it's likely HTML
}
There was a problem hiding this comment.
We analyzed this, and the current check is sufficient to validate html.
Testing <span> text </text><strong> Rahul
Invalid HTML, text not starting with a tag are all rendered by browser correctly and will not pass the aforementioned validity function.
It's best to stay put with current implementation. Let me know if you disagree.
| }, | ||
| ); | ||
|
|
||
| const htmlTableData = [ |
There was a problem hiding this comment.
Can we please put this data in fixture folder as file?
`HTMLCell_spec.ts` to enhance maintainability and clarity.
|
This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
app/client/cypress/fixtures/htmlCellInTableWidgetV2.ts (1)
1-11: Consider adding TypeScript interface for data structure.The test data structure would benefit from a TypeScript interface to ensure type safety and provide better documentation.
interface UserData { id: number; name: string; email: string; role?: string; status: string | null; applicationDate: string; lastUpdated: string; department: string; } export const htmlTableData: UserData[] = [...]
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/HTMLCell_spec.ts(1 hunks)app/client/cypress/fixtures/htmlCellInTableWidgetV2.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/HTMLCell_spec.ts
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/fixtures/htmlCellInTableWidgetV2.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 (1)
app/client/cypress/fixtures/htmlCellInTableWidgetV2.ts (1)
1-56: LGTM! The test data structure is comprehensive.
The fixture provides a good variety of test cases including:
- Undefined and null values
- Different status states with visual indicators
- Consistent date formats
- Various departments and roles
rahulbarwal
deleted the
rahulbarwal/Issue13681/Implementation/HTML-Column-type-in-table
branch
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Description
Adds support for rendering HTML content in Table Widget V2 columns with proper sanitization and filtering capabilities.
Fixes #13681
or
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Table"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12270923512
Commit: 5f873fc
Cypress dashboard.
Tags:
@tag.TableSpec:
Wed, 11 Dec 2024 07:23:01 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Release Notes
New Features
HTMLCellcomponent.Bug Fixes
Tests
HTMLCellcomponent and its interaction with HTML content.