bito-code-review[bot] commented on code in PR #44665:
URL: https://github.com/apache/superset/pull/44665#discussion_r4157450883
##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -1188,37 +1264,59 @@ export default function TableChart<D extends DataRecord
= DataRecord>(
};
`;
- const cellBarStyles = css`
- position: absolute;
- height: 100%;
- display: block;
- top: 0;
- ${
- valueRange &&
- typeof value === 'number' &&
- valueRangeFlag &&
- `
- width: ${`${cellWidth({
- value: value as number,
- valueRange,
+ // Whether this particular cell draws a bar. A CELL_BAR rule can
match
+ // some cells of a column and not others, so the bar — and with it
the
+ // click-to-filter suppression, which exists because the bar overlay
+ // would swallow the click — has to be decided per cell rather than
+ // from the column-wide valueRange.
+ const cellDrawsBar =
+ (generalShowCellBars ? !!valueRange : false) ||
+ !!backgroundColorCellBar;
+
+ // A styled component rather than the `css` prop or an inline
+ // `style`: the `css` prop needs the emotion JSX pragma, which this
+ // codebase's own Jest/Babel config does not wire up, and an inline
+ // style would outrank every dashboard stylesheet. Both matter here —
+ // the `cell-bar` classes exist precisely so saved custom CSS can
+ // restyle the bar, and a `styled` div keeps that override working
+ // while still emitting a real rule a test can read back.
+ // A background rule paints the cell itself, so the bar the global
+ // toggle would draw over that background is dropped. A CELL_BAR rule
+ // matching the same cell is a separate instruction and keeps its own
+ // geometry: clearing the flag for the background rule must not
flatten
+ // a cell bar into the full-width band, which would make every
matching
+ // value read as the same magnitude.
+ const barHasGeometry =
+ !!valueRange &&
+ numericValue !== undefined &&
+ (valueRangeFlag || !!backgroundColorCellBar);
+ const barGeometry = barHasGeometry
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Union type compile error</b></div>
<div id="fix">
`barGeometry` is inferred as `{width;left;backgroundColor} |
{width;left;backgroundColor} | {}`. Accessing
`barGeometry.width`/`.left`/`.backgroundColor` (lines 1426-1432) on a union
containing `{}` is a TS error — property access requires the property on every
member. Add an explicit type annotation to the object so the `{}` branch is
typed with optional fields.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
const barGeometry: {
width?: string;
left?: string | number;
backgroundColor?: string;
} = barHasGeometry
````
</div>
</details>
</div>
<small><i>Code Review Run #b79ca3</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
