bito-code-review[bot] commented on code in PR #34995:
URL: https://github.com/apache/superset/pull/34995#discussion_r2319729873
##########
superset-frontend/packages/superset-ui-core/src/components/Icons/BaseIcon.tsx:
##########
@@ -22,7 +22,7 @@ import { AntdIconType, BaseIconProps, CustomIconType,
IconType } from './types';
const genAriaLabel = (fileName: string) => {
const name = fileName.replace(/_/g, '-'); // Replace underscores with dashes
- const words = name.split(/(?=[A-Z])/); // Split at uppercase letters
+ const words = name.split(/(?<=[a-z])(?=[A-Z])/); // Split at
lowercase-to-uppercase transitions
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Regex change breaks aria-label generation</b></div>
<div id="fix">
The regex change from `/(?=[A-Z])/.filter(word => word.length > 0)` to
`/(?<=[a-z])(?=[A-Z])/` fundamentally alters string splitting behavior and
breaks aria-label generation for strings with consecutive uppercase letters
like 'SLACK', 'XMLHttpRequest', 'HTMLElement'. This will cause incorrect
accessibility labels in production. Please revert to the original regex pattern.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```suggestion
const words = name.split(/(?=[A-Z])/).filter(word => word.length > 0); //
Split at uppercase letters and filter out empty strings
```
</div>
</details>
</div>
<small><i>Code Review Run <a
href=https://github.com/apache/superset/pull/34995#issuecomment-3250166338>#c3ee1f</a></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/src/features/alerts/AlertReportModal.test.tsx:
##########
@@ -93,8 +93,9 @@ const generateMockPayload = (dashboard = true) => {
...mockPayload,
chart: {
id: 1,
- slice_name: 'Test Chart',
- viz_type: VizType.Table,
+ slice_name: 'test chart',
+ viz_type: 'table',
+ value: 1,
},
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing required label property in chart mock</b></div>
<div id="fix">
The chart object mock is missing the required `label` property. The
AlertReportModal component expects chart objects to have a `label` property as
defined in the MetaObject interface, and the component code specifically checks
for `chart.label` in multiple places (lines 871, 889, 1646, 1650). Add `label:
'test chart'` to the chart object to fix the broken functionality.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```suggestion
chart: {
id: 1,
label: 'test chart',
slice_name: 'test chart',
viz_type: 'table',
value: 1,
},
```
</div>
</details>
</div>
<small><i>Code Review Run <a
href=https://github.com/apache/superset/pull/34995#issuecomment-3250166338>#c3ee1f</a></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: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
