bito-code-review[bot] commented on code in PR #44147:
URL: https://github.com/apache/superset/pull/44147#discussion_r4174014700
##########
superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts:
##########
@@ -425,6 +428,87 @@ export default function transformProps(
{},
);
+ // Wraps a single line of text by pixel width, breaking at spaces. Oversized
+ // tokens (e.g. long identifiers or CJK strings without spaces) are split
+ // character-by-character so they never overflow the configured limit.
+ const wrapLine = (line: string, maxWidth: number): string => {
+ const words = line.split(' ');
+ const resultLines: string[] = [];
+ let currentLine = '';
+
+ const pushWord = (token: string) => {
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Redundant measureText calls</b></div>
<div id="fix">
`pushWord` measures every token twice (`measureTextWidth(token)` then the
full `testLine`) and the char loop re-measures each growing prefix, so long
labels cost O(n²) canvas `measureText` calls per token on the formatter hot
path; the LRU cache in `measureTextWidth` (`utils/series.ts`, cap 2000) only
absorbs repeats. Consider measuring word widths once and extending
incrementally.
</div>
</div>
<small><i>Code Review Run #ac7a1e</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
##########
superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts:
##########
@@ -425,6 +428,87 @@ export default function transformProps(
{},
);
+ // Wraps a single line of text by pixel width, breaking at spaces. Oversized
+ // tokens (e.g. long identifiers or CJK strings without spaces) are split
+ // character-by-character so they never overflow the configured limit.
+ const wrapLine = (line: string, maxWidth: number): string => {
+ const words = line.split(' ');
+ const resultLines: string[] = [];
+ let currentLine = '';
+
+ const pushWord = (token: string) => {
+ // If the token itself is wider than maxWidth, split char by char.
+ if (measureTextWidth(token, theme) > maxWidth) {
+ if (currentLine) {
+ resultLines.push(currentLine);
+ currentLine = '';
+ }
+ let charBuf = '';
+ for (const ch of token) {
+ const testBuf = `${charBuf}${ch}`;
+ if (measureTextWidth(testBuf, theme) <= maxWidth) {
+ charBuf = testBuf;
+ } else {
+ if (charBuf) resultLines.push(charBuf);
+ charBuf = ch;
+ }
+ }
+ if (charBuf) currentLine = charBuf;
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Dropped char on overflow</b></div>
<div id="fix">
In `wrapLine`'s oversized-token loop, when a token's final character
overflows, the else branch flushes it to `resultLines` and leaves `charBuf`
empty, so line 456 resets `currentLine` to `''` and the flushed fragment is
stranded while the next word starts a fresh line — the character is dropped
from the flow. Initialize `currentLine = charBuf` unconditionally so the
trailing fragment carries over.
</div>
</div>
<small><i>Code Review Run #ac7a1e</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]
