bito-code-review[bot] commented on code in PR #42009:
URL: https://github.com/apache/superset/pull/42009#discussion_r3573632782
##########
superset-frontend/src/features/datasets/AddDataset/DatasetPanel/DatasetPanel.tsx:
##########
@@ -235,6 +236,11 @@ const renderExistingDatasetAlert = (dataset?:
DatasetObject) => (
openInNewTab(stripAppRoot(dataset.explore_url));
}
}}
+ onKeyDown={handleKeyboardActivation(() => {
+ if (dataset?.explore_url) {
+ openInNewTab(stripAppRoot(dataset.explore_url));
+ }
+ })}
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Semantic duplication in event handlers</b></div>
<div id="fix">
The `onClick` at line 231-238 and the `onKeyDown` callback at line 239-243
are identical. Extract to `const openExplore = () => { if
(dataset?.explore_url) { openInNewTab(stripAppRoot(dataset.explore_url)); } };`
to eliminate duplication and prevent future divergence.
</div>
</div>
<small><i>Code Review Run #4da197</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/ListView/ListView.tsx:
##########
@@ -255,6 +256,7 @@ const ViewModeToggle = ({
e.currentTarget.blur();
setMode('card');
}}
+ onKeyDown={handleKeyboardActivation(() => setMode('card'))}
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing keyboard accessibility tests</b></div>
<div id="fix">
The `handleKeyboardActivation` handlers added on lines 259, 274, 515-517,
and 553-555 are correctly wired, but the existing test suite (363 lines, 15
tests) has zero coverage for keyboard event handling. Since
`handleKeyboardActivation` is a utility designed to make interactive elements
keyboard-accessible (Enter/Space), the corresponding test file should include
at least one test that exercises the keyboard event path and asserts the
resulting state change.
</div>
</div>
<small><i>Code Review Run #4da197</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]
