Repository navigation
feat(table): add conditional formatting support for categorical columns - #37756
sharmavikas18 wants to merge 4 commits into
Conversation
Code Review Agent Run #b05187Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
There was a problem hiding this comment.
Pull request overview
Enables conditional formatting for categorical (String/Boolean) columns in the Standard Table and AG Grid Table plugins by broadening the conditional-formatting column filtering logic and updating control descriptions.
Changes:
- Updated conditional formatting control description to be type-agnostic (“…to columns”) in both table plugins.
- Expanded conditional-formatting column eligibility to include
GenericDataType.StringandGenericDataType.Boolean(previously numeric-only / time-compare gated).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx | Broadens eligible conditional-formatting columns and updates help text. |
| superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx | Broadens eligible conditional-formatting columns and updates help text. |
Comments suppressed due to low confidence (2)
superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx:820
- When
hasTimeComparisonis true,columnOptionsis derived viaprocessComparisonColumns(numericColumns, ...). Now thatnumericColumnsincludes String/Boolean, this path will (a) generateMain/#/△/%options for categorical columns that do not exist intransformProps, (b) drop the original categorical columns from the selector, and (c) losedataType, causing the conditional-formatting UI to fall back to numeric operators. Consider splitting numeric vs categorical options: generate comparison options only for numeric columns (withdataType: Numeric), and include String/Boolean columns as-is so they remain selectable during time comparison.
const numericColumns =
Array.isArray(colnames) && Array.isArray(coltypes)
? colnames.reduce((acc, colname, index) => {
if (
coltypes[index] === GenericDataType.Numeric ||
coltypes[index] === GenericDataType.String ||
coltypes[index] === GenericDataType.Boolean
) {
acc.push({
value: colname,
label: Array.isArray(verboseMap)
? colname
: (verboseMap[colname] ?? colname),
dataType: coltypes[index],
});
}
return acc;
}, [])
: [];
const columnOptions = hasTimeComparison
? processComparisonColumns(
numericColumns || [],
ensureIsArray(timeCompareValue)[0]?.toString() || '',
)
: numericColumns;
superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx:739
- In time comparison mode,
processComparisonColumns(numericColumns, ...)will now receive String/Boolean columns too. That helper generatesMain/#/△/%variants and omitsdataType, which means categorical columns either disappear (originals not included) or show invalid comparison-prefixed options with numeric operators. Update the time-comparison branch to only expand numeric columns into comparison options (withdataType: Numeric) and keep categorical columns unchanged incolumnOptions.
const numericColumns =
Array.isArray(colnames) && Array.isArray(coltypes)
? colnames
.filter(
(colname: string, index: number) =>
coltypes[index] === GenericDataType.Numeric ||
coltypes[index] === GenericDataType.String ||
coltypes[index] === GenericDataType.Boolean,
)
.map((colname: string) => ({
value: colname,
label: Array.isArray(verboseMap)
? colname
: (verboseMap[colname] ?? colname),
dataType:
colnames && coltypes[colnames?.indexOf(colname)],
}))
: [];
const columnOptions = hasTimeComparison
? processComparisonColumns(
numericColumns || [],
ensureIsArray(timeCompareValue)[0]?.toString() || '',
)
: numericColumns;
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #446e9aActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
Superset uses Git pre-commit hooks courtesy of pre-commit. To install run the following: pip3 install -r requirements/development.txt Alternatively it is possible to run pre-commit by running pre-commit manually: pre-commit run --all-files |
- theming.mdx: document ECharts array property overrides (PR #37965) — array values like color palettes are fully supported and replaced entirely (not merged); adds Array Property Overrides section with color palette example - configuring-superset.mdx: document PKCE support for database OAuth2 (PR #37067) — add PKCE section under Custom OAuth2 with code_challenge_method config and when to use it - cache.mdx: document ETag support for thumbnail/screenshot endpoints (PR #37663) — conditional GET with If-None-Match to avoid downloading unchanged images - exploring-data.mdx: document SQL Lab UX improvements (PRs #37298, #37694, #37756) — treeview table browser, Ctrl+F find widget, resizable panels; also adds time range natural language expressions section (PR #37098) - creating-your-first-dashboard.mdx: document Table chart features — boolean and categorical conditional formatting (PRs #36338, #37756), gradient toggle (PR #36280), HTML cell rendering with security note (PR #37685), column header tooltips from dataset descriptions (PR #37179), Display Controls modal in dashboard view (PR #36062) - databases.json: update StarRocks supports_catalog and supports_dynamic_catalog to true — the engine spec (PR #37026) already implemented full catalog support with get_catalog_names(), get_default_catalog(), and SHOW CATALOGS; the committed JSON was stale and did not reflect this Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
This feature needs to be included. Please add conditional formatting to categorical or boolean columns such as strings, etc. |
|
Thanks @sharmavikas18, but I think this got superseded by #34330, which is why I closed the linked issue. Going to close this out, but holler if I'm mistakens or there's still a gap, and we can reopen/revisit this, no problem. |
|
Well... reopening. Feeling like I might have missed some subtlety there. Feel free to see if you agree or not with my prior statement. That said, this needs a rebase if it's going to merge and is indeed needed. |
|
Attempting a rebase to see how this shakes out... |
14251b9 to
cdd8ead
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #37756 +/- ##
==========================================
- Coverage 84.63% 84.62% -0.01%
==========================================
Files 3069 3069
Lines 205446 205449 +3
Branches 48306 48310 +4
==========================================
- Hits 173879 173870 -9
- Misses 28277 28285 +8
- Partials 3290 3294 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Review Agent Run #db6f05Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
@rusackas let me know if you need me to make any changes |
github-actions
Bot
added
the
requires:rebase
|
Confirmed #34330 didn't cover this — master still blocks String/Boolean columns from conditional formatting under time comparison. Just needs a rebase to land. |
…s and add time comparison guard
cdd8ead to
6af3615
Compare
github-actions
Bot
added
risk:chart-metadata
and removed
requires:rebase
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #3f5d00
Actionable Suggestions - 1
-
superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx - 1
- Duplicated column mapping · Line 884-891
Review Details
-
Files reviewed - 2 · Commit Range:
8b8be67..6af3615- superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx
- superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers an incremental AI Review. -
/review full- Manually triggers a full AI Review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
| ? colnames | ||
| .filter( | ||
| (colname: string, index: number) => | ||
| coltypes[index] === GenericDataType.Numeric, | ||
| ) | ||
| .map((colname: string) => ({ | ||
| .map((colname: string, index: number) => ({ | ||
| value: colname, | ||
| label: Array.isArray(verboseMap) | ||
| ? colname | ||
| : (verboseMap?.[colname] ?? colname), | ||
| // Every entry here already passed the Numeric filter | ||
| // above, so the type is always Numeric — no need to | ||
| // re-look it up (which breaks on duplicate colnames). | ||
| dataType: GenericDataType.Numeric, | ||
| dataType: coltypes[index], | ||
| })) |
There was a problem hiding this comment.
The new eligibleColumns map (884-891) duplicates the identical colnames.map(...) mapping already in allColumns (866-872). Two parallel mappings in the same mapStateToProps can silently diverge when one is updated. Consider a shared helper or deriving eligibleColumns from allColumns (minus the ENTIRE_ROW entry) before filtering by type.
Code Review Run #3f5d00
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
|
Looks like there are still issues with CI. I'll see if I can help move this forward again, but please feel free to jump in and/or revert my commits if you'd rather tackle things yourself, @sharmavikas18. |
The categorical-formatting change reworded the conditional-formatting description string without regenerating messages.pot, failing babel-extract's drift check. It also changed eligibleColumns to include String/Boolean columns alongside Numeric ones when there is no time comparison, which the duplicate-colname regression test hadn't been updated to expect. Co-Authored-By: Evan Rusackas <evan@preset.io> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Code Review Agent Run #6011bdActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
Fixes #37678
SUMMARY
This PR enables conditional formatting for categorical columns (String and Boolean)
in both the Standard Table plugin and the AG Grid Table plugin.
Previously, categorical columns were blocked by control panel filtering logic,
even though the conditional formatting UI already supported categorical operators.
This change removes those restrictions and allows categorical columns to be used
in conditional formatting rules.
Changes made:
Standard Table plugin
File:
superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
!hasTimeComparisonrestriction that prevented String and Booleancolumns from appearing in the conditional formatting selector.
"Apply conditional color formatting to numeric columns"
to:
"Apply conditional color formatting to columns".
AG Grid Table plugin
File:
superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx
in addition to GenericDataType.Numeric.
User impact:
equals, begins with, ends with, contains, not contains
is true, is false, is null, is not null
Technical notes:
GenericDataTypeenum.All relevant tests passed (254 tests).
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable.
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION