korbit-ai[bot] commented on code in PR #32665:
URL: https://github.com/apache/superset/pull/32665#discussion_r1995190673
##########
superset-frontend/plugins/plugin-chart-table/src/types.ts:
##########
@@ -56,6 +56,7 @@ export interface DataColumnMeta {
key: string;
// `label` is verbose column name used for rendering
label: string;
+ originalLabel?: string;
Review Comment:
### Unclear relationship between label properties <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The new property `originalLabel` lacks documentation explaining its purpose
and relationship with the existing `key` and `label` properties.
###### Why this matters
Without clear documentation on the differences between `key`, `label`, and
`originalLabel`, developers may use them incorrectly or struggle to maintain
code consistency.
###### Suggested change ∙ *Feature Preview*
// `key` is what is called `label` in the input props
key: string;
// `label` is verbose column name used for rendering
label: string;
// `originalLabel` preserves the initial label value before any
transformations
originalLabel?: string;
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/836d847c-75b9-4da6-860d-e3d3ad11e5ba?suggestedFixEnabled=true)
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:4e8f10d3-5ff2-4159-9c41-422a14d92a90 -->
##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -605,7 +605,9 @@ export default function TableChart<D extends DataRecord =
DataRecord>(
// Calculate the number of placeholder columns needed before the current
header
const startPosition = value[0];
const colSpan = value.length;
-
+ // Retrieve the originalLabel from the first column in this group
+ const originalLabel = columnsMeta[value[0]]?.originalLabel || key;
Review Comment:
### Missing error logging for originalLabel fallback <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The code silently falls back to using 'key' when columnsMeta[value[0]] is
undefined or originalLabel is missing, without logging the failure.
###### Why this matters
In case of data inconsistency, debugging would be difficult as there's no
indication of why the originalLabel wasn't available.
###### Suggested change ∙ *Feature Preview*
```typescript
// Add error logging before fallback
const originalLabel = columnsMeta[value[0]]?.originalLabel || (() => {
console.warn(`Missing originalLabel for column at index ${value[0]},
falling back to key: ${key}`);
return key;
})();
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d71f2fa4-23fe-4040-b0f1-c75b07471cf4?suggestedFixEnabled=true)
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:cdc51218-bb2f-42fc-b3a5-c947820a6888 -->
--
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]
