kokhlo commented on code in PR #44665:
URL: https://github.com/apache/superset/pull/44665#discussion_r4138410307


##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -1050,11 +1084,29 @@ export default function TableChart<D extends DataRecord 
= DataRecord>(
         basicColorFormatters.length > 0;
       const generalShowCellBars =
         config.showCellBars === undefined ? showCellBars : config.showCellBars;
+      // A Cell bar conditional-formatting rule must keep working even when the
+      // generic "Show cell bars" toggle is off: the toggle controls the 
default
+      // gradient, not whether an explicit formatter rule can draw its bar.

Review Comment:
   Both places you flagged are the same decision, and the tooltip was the one 
place still describing the old contract. The cell-bar rule does draw with the 
global toggle off, so the Explore tooltip now says that: a matching cell draws 
a bar on its own, and enabling "Show cell bars" additionally draws bars across 
the column. Updated in `4781e85`.



##########
superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx:
##########
@@ -50,6 +50,71 @@ import transformProps from '../src/transformProps';
 import testData from './testData';
 import { ProviderWrapper } from './testHelpers';
 
+// The bar's width comes from an emotion class, not an inline style, so it has
+// to be read back out of the stylesheet the rule actually generated.

Review Comment:
   Right, the comment described the pre-inline-style implementation. The width 
is now written as an inline `style` percentage, and the helper reads it back 
off the element to tell a scaled bar from the full-width fallback band. Comment 
updated in `4781e85`; the helper logic itself is unchanged.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to