codeant-ai-for-open-source[bot] commented on code in PR #37304:
URL: https://github.com/apache/superset/pull/37304#discussion_r2743075839
##########
superset-frontend/src/components/ListView/ListView.tsx:
##########
@@ -272,7 +277,7 @@ export function ListView<T extends object = any>({
filters = [],
bulkActions = [],
bulkSelectEnabled = false,
- disableBulkSelect = () => {},
+ disableBulkSelect = () => { },
Review Comment:
**Suggestion:** The function parameter default `disableBulkSelect = () => {
}` silently no-ops when the parent does not provide a handler, which will make
the BulkSelect "close" action ineffective; avoid providing a no-op default here
(leave it undefined) so callers must explicitly opt in or let the component
handle fallback behavior where `toggleAllRowsSelected(false)` can be used.
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Bulk-select close action may not disable bulk selection.
- ⚠️ UI state can become inconsistent after closing alert.
- ⚠️ Affects bulk-select workflow and controls visibility.
```
</details>
```suggestion
disableBulkSelect,
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Render a ListView with bulkSelectEnabled=true but without passing a
disableBulkSelect
prop from the parent. The function parameter default is defined in
superset-frontend/src/components/ListView/ListView.tsx at line 280
(`disableBulkSelect =
() => { },`).
2. The BulkSelectWrapper is shown because bulkSelectEnabled is true; the
wrapper is
rendered with onClose={disableBulkSelect} (ListView.tsx BulkSelectWrapper
usage).
3. Click the Alert close button. The onClose handler executes the default
no-op
(ListView.tsx:280) which does nothing, so the parent bulk select state (the
prop
controlling whether the bulk controls should be visible) is not updated.
4. Result: the close action either visually reappears on next render or
leaves internal
state inconsistent because the parent was never informed. If the parent
expects to be
notified to turn off bulkSelectEnabled, that will never happen. If this
default was
intentional, it hides missing handler bugs; otherwise it produces confusing
behavior.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/components/ListView/ListView.tsx
**Line:** 280:280
**Comment:**
*Logic Error: The function parameter default `disableBulkSelect = () =>
{ }` silently no-ops when the parent does not provide a handler, which will
make the BulkSelect "close" action ineffective; avoid providing a no-op default
here (leave it undefined) so callers must explicitly opt in or let the
component handle fallback behavior where `toggleAllRowsSelected(false)` can be
used.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
superset-frontend/src/components/ListView/ListView.tsx:
##########
@@ -463,8 +469,8 @@ export function ListView<T extends object = any>({
current={pageIndex + 1}
pageSize={pageSize}
total={count}
- onChange={page => {
- gotoPage(page - 1);
+ onChange={(page: number) => {
+ setPage(page);
Review Comment:
**Suggestion:** The Pagination onChange handler only updates a local state
(`setPage`) and does not call the table pagination controller (`gotoPage`), so
clicking pages will not change the displayed page or trigger data fetch; change
the handler to call `gotoPage(page - 1)` (Antd Pagination is 1-based) so the
list actually navigates. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Card view pagination doesn't navigate or fetch data.
- ⚠️ Users stuck on first page in card-mode lists.
- ⚠️ Affects any ListView rendering CardCollection pagination.
```
</details>
```suggestion
gotoPage(page - 1);
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the application and navigate to a ListView rendered in card (grid)
mode. The card
pagination UI is rendered in
superset-frontend/src/components/ListView/ListView.tsx at the
pagination block (lines 468-474) where the Antd <Pagination> component is
configured.
2. Inspect the ListView component state: ListView destructures gotoPage from
the
useListViewState hook
(superset-frontend/src/components/ListView/ListView.tsx: line 302)
which is the function that actually changes pageIndex and triggers data
fetches.
3. Click any page number in the card-mode pagination control. The onChange
handler at
superset-frontend/src/components/ListView/ListView.tsx:472-474 is executed;
it currently
calls setPage(page) (state setter declared at ListView.tsx:294) instead of
calling
gotoPage.
4. Because gotoPage is not invoked, the pageIndex in useListViewState does
not change and
the fetchData effect in superset-frontend/src/components/ListView/utils.ts
(fetchData call
at utils.ts:346) is not triggered — the visible items remain on the same
page instead of
navigating.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/components/ListView/ListView.tsx
**Line:** 473:473
**Comment:**
*Logic Error: The Pagination onChange handler only updates a local
state (`setPage`) and does not call the table pagination controller
(`gotoPage`), so clicking pages will not change the displayed page or trigger
data fetch; change the handler to call `gotoPage(page - 1)` (Antd Pagination is
1-based) so the list actually navigates.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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]
