bito-code-review[bot] commented on code in PR #41184:
URL: https://github.com/apache/superset/pull/41184#discussion_r3565630723
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx:
##########
@@ -94,7 +94,6 @@ interface SubtotalOptions {
interface TableRendererProps {
cols: string[];
rows: string[];
- aggregatorName: string;
tableOptions?: TableOptions;
Review Comment:
<!-- Bito Reply -->
The suggestion to reintroduce `aggregatorName` is not necessary in this
context. As you noted, the removal of the per-metric aggregation function
control means this prop is no longer used for client-side selection, and
keeping it would introduce unused code.
**superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx**
```
interface TableRendererProps {
cols: string[];
rows: string[];
tableOptions?: TableOptions;
```
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx:
##########
@@ -1099,9 +1096,7 @@ export function TableRenderer(props: TableRendererProps) {
true,
)}
>
- {t('Total (%(aggregatorName)s)', {
- aggregatorName: t(aggregatorName),
- })}
+ {t('Total')}
Review Comment:
<!-- Bito Reply -->
The suggestion to restore the translation pattern is not necessary in this
context. As noted, the aggregation function control has been removed, making
the simplified 'Total' label appropriate for the current implementation.
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.tsx:
##########
@@ -1448,9 +1437,7 @@ export function TableRenderer(props: TableRendererProps) {
true,
)}
>
- {t('Total (%(aggregatorName)s)', {
- aggregatorName: t(aggregatorName),
- })}
+ {t('Total')}
Review Comment:
<!-- Bito Reply -->
The suggestion to restore the `aggregatorName` in the total column header is
a review comment, not a mandatory requirement. If the `aggregatorName` control
has been intentionally removed as part of the changes described in
`UPDATING.md`, then the suggestion is not applicable to the current state of
the code. You may choose to ignore this suggestion if it conflicts with the
intended design changes.
--
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]
