Re: [PR] fix(table-chart): fix missing table header IDs [superset]

Mon, 03 Nov 2025 21:28:21 -0800 


korbit-ai[bot] commented on code in PR #35968:
URL: https://github.com/apache/superset/pull/35968#discussion_r2488754060


##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -844,6 +858,9 @@ export default function TableChart<D extends DataRecord = 
DataRecord>(
         }
       }
 
+      // Cache sanitized header ID to avoid recomputing it multiple times
+      const headerId = sanitizeHeaderId(column.originalLabel || column.key);

Review Comment:
   ### Redundant header ID sanitization in render loop <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The sanitizeHeaderId function is called inside the getColumnConfigs callback 
which is executed for every column during rendering, causing unnecessary 
recomputation of the same header ID multiple times.
   
   
   ###### Why this matters
   This leads to redundant string processing operations during table rendering, 
especially when the table has many columns or re-renders frequently, impacting 
performance.
   
   ###### Suggested change ∙ *Feature Preview*
   Move the header ID sanitization outside the render callback by pre-computing 
sanitized IDs when columns are processed, or memoize the sanitizeHeaderId 
function with useMemo to cache results based on the input string.
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/287d0344-7b0f-48c1-bf39-0ee61891e2ac/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/287d0344-7b0f-48c1-bf39-0ee61891e2ac?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/287d0344-7b0f-48c1-bf39-0ee61891e2ac?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/287d0344-7b0f-48c1-bf39-0ee61891e2ac?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/287d0344-7b0f-48c1-bf39-0ee61891e2ac)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:71295609-01ee-487a-ba1a-c41a10868fa0 -->
   
   
   [](71295609-01ee-487a-ba1a-c41a10868fa0)



##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -844,6 +858,9 @@ export default function TableChart<D extends DataRecord = 
DataRecord>(
         }
       }
 
+      // Cache sanitized header ID to avoid recomputing it multiple times
+      const headerId = sanitizeHeaderId(column.originalLabel || column.key);

Review Comment:
   ### Inconsistent header ID generation fallback <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The sanitizeHeaderId function is called with a fallback that may result in 
inconsistent ID generation when column.originalLabel is undefined or null but 
column.key exists.
   
   
   ###### Why this matters
   This could lead to accessibility issues where aria-labelledby references may 
not match the actual header IDs, breaking screen reader functionality and ARIA 
relationships.
   
   ###### Suggested change ∙ *Feature Preview*
   Ensure consistent fallback logic by using a more explicit approach:
   ```typescript
   const headerId = sanitizeHeaderId(column.originalLabel ?? column.key ?? '');
   ```
   Or add validation to ensure the input is always a string:
   ```typescript
   const columnIdentifier = column.originalLabel || column.key || `col-${i}`;
   const headerId = sanitizeHeaderId(columnIdentifier);
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4fca14ad-f411-4736-af55-92780ddfd4e6/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4fca14ad-f411-4736-af55-92780ddfd4e6?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4fca14ad-f411-4736-af55-92780ddfd4e6?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4fca14ad-f411-4736-af55-92780ddfd4e6?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4fca14ad-f411-4736-af55-92780ddfd4e6)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:e5e39b6a-f4a8-41e0-8d94-86c7f015a329 -->
   
   
   [](e5e39b6a-f4a8-41e0-8d94-86c7f015a329)



-- 
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