bito-code-review[bot] commented on code in PR #41937:
URL: https://github.com/apache/superset/pull/41937#discussion_r3581164936
##########
superset-frontend/src/components/Chart/DrillDetail/DrillDetailTableControls.stories.tsx:
##########
@@ -33,4 +33,9 @@ InteractiveTableControls.args = {
{ op: '>', col: 'tz_offset', val: 200 },
{ op: '==', col: 'platform', val: 'GB' },
],
+ canDownload: true,
+ onDownloadCSV: () => {},
+ onDownloadXLSX: () => {},
+ onReload: () => {},
+ loading: false,
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing required prop</b></div>
<div id="fix">
The `setFilters` prop is required by `TableControlsProps` (line 37 of
DrillDetailTableControls.tsx) but is missing from the story args. Without it,
the component will crash when users interact with filter removal buttons since
`removeFilter` at line 74 calls `setFilters()`. Add `setFilters: () => {}`
alongside the other callback props.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
```
</div>
</details>
</div>
<small><i>Code Review Run <a
href=https://github.com/apache/superset/pull/41937#issuecomment-4971919105>#c2d5ea</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/components/Chart/DrillDetail/DrillDetailTableControls.tsx:
##########
@@ -118,16 +132,28 @@ export default function TableControls({
display: flex;
align-items: center;
height: min-content;
+ gap: ${theme.sizeUnit * 3}px;
`}
>
<RowCountLabel loading={loading && !totalCount} rowcount={totalCount}
/>
- <Icons.ReloadOutlined
- iconColor={theme.colorIcon}
- iconSize="l"
- aria-label={t('Reload')}
- role="button"
- onClick={onReload}
- />
+ {canDownload && (
+ <DownloadDropdown
+ onDownloadCSV={onDownloadCSV}
+ onDownloadXLSX={onDownloadXLSX}
+ />
+ )}
+ {canDownload && (
+ <CopyToClipboardButton data={data} columns={columnNames} />
+ )}
+ <Tooltip title={t('Reload')}>
+ <Icons.ReloadOutlined
+ iconColor={theme.colorIcon}
+ iconSize="l"
+ aria-label={t('Reload')}
+ role="button"
+ onClick={onReload}
+ />
+ </Tooltip>
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Keyboard accessibility missing on reload button</b></div>
<div id="fix">
The Tooltip-wrapped reload button lacks `tabIndex` and an explicit keyboard
handler on the inner element. Without `tabIndex={0}`, non-interactive icon
elements are skipped during keyboard navigation. Compare with
`DownloadDropdown` (line 60-65) which correctly wraps the icon in `<span
role="button" tabIndex={0}>`. Keyboard activation (Enter/Space) won't trigger
`onClick` without these attributes.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
```
</div>
</details>
</div>
<small><i>Code Review Run <a
href=https://github.com/apache/superset/pull/41937#issuecomment-4971919105>#c2d5ea</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: [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]
