bito-code-review[bot] commented on code in PR #38342:
URL: https://github.com/apache/superset/pull/38342#discussion_r2874156078
##########
superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.tsx:
##########
@@ -250,8 +251,10 @@ const RawTableView = ({
}
}, [initialState.sortBy, onServerPagination, serverPagination, sortBy]);
+ const ariaLabel = props['aria-label'] ?? 'Data table';
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing translation for aria-label default</b></div>
<div id="fix">
The default 'Data table' string in the aria-label should be translatable for
internationalization. Wrap it with the t() function from @superset-ui/core to
ensure it's extracted for translation.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
---
a/superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.tsx
+++
b/superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.tsx
@@ -19,6 +19,7 @@ import { memo, useEffect, useRef, useMemo, useCallback }
from 'react';
import { isEqual } from 'lodash';
import { styled } from '@apache-superset/core/ui';
import { useFilters, usePagination, useSortBy, useTable } from
'react-table';
+import { t } from '@superset-ui/core';
import { Empty } from '@superset-ui/core/components';
import TableCollection from '@superset-ui/core/components/TableCollection';
import { TableSize } from '@superset-ui/core/components/Table';
@@ -254,7 +255,7 @@ const RawTableView = ({
}, [initialState.sortBy, onServerPagination, serverPagination, sortBy]);
const ariaLabel = props['aria-label'] ?? 'Data table';
+ const ariaLabel = props['aria-label'] ?? t('Data table');
return (
<TableViewStyles {...props} ref={tableRef} role="region"
aria-label={ariaLabel}>
```
</div>
</details>
</div>
<small><i>Code Review Run #c95bd5</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/pages/Register/index.tsx:
##########
@@ -182,7 +182,7 @@ export default function Login() {
label={t('Password')}
name="password"
rules={[
- { required: true, message: t('Please enter your password') },
+ { required: true, message: t('Please enter a password (minimum 8
characters)') },
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing password minimum length validation</b></div>
<div id="fix">
The password validation message specifies a minimum of 8 characters, but
only required validation is implemented. This could allow weak passwords and
mislead users about security requirements. Adding the min validator ensures
consistency between message and behavior.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
{ required: true, min: 8, message: t('Please enter a password
(minimum 8 characters)') },
````
</div>
</details>
</div>
<small><i>Code Review Run #c95bd5</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/dashboard/components/nativeFilters/FilterBar/FilterControls/GroupByFilterCard.tsx:
##########
@@ -156,6 +156,18 @@ const HorizontalFormItem = styled(FormItem)`
min-width: 200px !important;
max-width: 400px !important;
}
+
+ /* WCAG 1.4.10 Reflow: full-width filter controls at narrow viewports */
+ @media (max-width: 480px) {
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>WCAG 1.4.10 Reflow Breakpoint</b></div>
<div id="fix">
The media query uses max-width: 480px for reflow, but WCAG 1.4.10 requires
content to reflow without horizontal scrolling at 320px width. While 480px is
consistent with other components in the codebase, updating to 320px would
ensure full compliance with the accessibility standard mentioned in the comment.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
@media (max-width: 320px) {
````
</div>
</details>
</div>
<small><i>Code Review Run #c95bd5</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/userInfo/UserInfoModal.tsx:
##########
@@ -91,10 +92,11 @@ function UserInfoModal({
<FormItem
name="password"
label={t('Password')}
- rules={[{ required: true, message: t('Password is required') }]}
+ rules={[{ required: true, message: t('New password is required
(minimum 8 characters)') }]}
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Misleading password validation message</b></div>
<div id="fix">
The validation message for the password field now mentions 'minimum 8
characters', but there's no corresponding client-side validation rule to
enforce this. This could lead to user confusion if they enter shorter passwords
expecting them to be accepted. Adding a min length rule would align the
validation with the message.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
rules={[{ required: true, message: t('New password is required
(minimum 8 characters)') }, { min: 8, message: t('Password must be at least 8
characters') }]
````
</div>
</details>
</div>
<small><i>Code Review Run #c95bd5</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/packages/superset-ui-core/src/components/Dropdown/index.tsx:
##########
@@ -90,7 +90,12 @@ export const MenuDotsDropdown = ({
...rest
}: MenuDotsDropdownProps) => (
<AntdDropdown popupRender={() => overlay} {...rest}>
- <MenuDotsWrapper data-test="dropdown-trigger">
+ <MenuDotsWrapper
+ data-test="dropdown-trigger"
+ role="button"
+ tabIndex={0}
+ aria-label="More actions"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Hardcoded user-facing text in aria-label</b></div>
<div id="fix">
The aria-label attribute uses hardcoded text 'More actions' that should be
internationalized. Per repository guidelines, all user-facing text labels must
be wrapped with the translation function t(). Add import { t } from
'@apache-superset/core'; and change aria-label="More actions" to
aria-label={t('More actions')}.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
- import { styled } from '@apache-superset/core/ui';
+ import { styled } from '@apache-superset/core/ui';
+ import { t } from '@apache-superset/core';
@@ -93,8 +93,8 @@
- <MenuDotsWrapper
- data-test="dropdown-trigger"
- role="button"
- tabIndex={0}
- aria-label="More actions"
- >
+ <MenuDotsWrapper
+ data-test="dropdown-trigger"
+ role="button"
+ tabIndex={0}
+ aria-label={t('More actions')}
+ >
```
</div>
</details>
</div>
<small><i>Code Review Run #c95bd5</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/pages/Register/index.tsx:
##########
@@ -129,7 +129,7 @@ export default function Login() {
label={t('Username')}
name="username"
rules={[
- { required: true, message: t('Please enter your username') },
+ { required: true, message: t('Please enter a username (3-150
characters)') },
]}
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing username length validation</b></div>
<div id="fix">
The username field validation message indicates a 3-150 character
requirement, but the form rules only check for required input. This mismatch
could confuse users expecting length validation. Adding min and max validators
will ensure the frontend enforces these constraints.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
rules={[
{ required: true, message: t('Please enter a username (3-150
characters)'), min: 3, max: 150 },
]}
````
</div>
</details>
</div>
<small><i>Code Review Run #c95bd5</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]
