You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "msyavuz (via GitHub)" <gi...@apache.org> on 2026/03/10 17:01:30 UTC
[PR] Msyavuz/chore/react 18 clean [superset]
msyavuz opened a new pull request, #38563:
URL: https://github.com/apache/superset/pull/38563
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
Upgrade to React 18
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2939308221
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx:
##########
@@ -298,10 +298,12 @@ const AgGridDataTable: FunctionComponent<AgGridTableProps> = memo(
};
const handleColumnHeaderClick = useCallback(
- params => {
+ (params: { column?: { colId?: string; sort?: string } }) => {
const colId = params?.column?.colId;
const sortDir = params?.column?.sort;
- handleColSort(colId, sortDir);
+ if (colId && sortDir) {
+ handleColSort(colId, sortDir);
+ }
Review Comment:
<!-- Bito Reply -->
This question isnโt related to the pull request. I can only help with questions about the PRโs code or comments.
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Msyavuz/chore/react 18 clean [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2913322230
##########
superset-frontend/src/features/reports/ReportModal/index.tsx:
##########
@@ -366,7 +367,7 @@ function ReportModal({
}}
onError={setCronError}
/>
- <StyledCronError>{cronError}</StyledCronError>
+ <StyledCronError>{cronError as ReactNode}</StyledCronError>
<div
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect cron error display</b></div>
<div id="fix">
Rendering the cronError object directly will show '[object Object]' instead of the error message. CronError from react-js-cron is an object with 'type' and 'description' properties - the description should be displayed.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
/>
<StyledCronError>{cronError?.description}</StyledCronError>
<div
````
</div>
</details>
</div>
<small><i>Code Review Run #139db3</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/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx:
##########
@@ -219,11 +221,14 @@ export default function getControlItemsMap({
}}
>
<>
- {controlItem.config.label}
+ {typeof controlItem.config.label === 'function'
+ ? (controlItem.config.label as Function)()
+ : controlItem.config.label}
+
{controlItem.config.description && (
<InfoTooltip
placement="top"
- tooltip={controlItem.config.description}
+ tooltip={controlItem.config.description as React.ReactNode}
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Inconsistent function handling for config properties</b></div>
<div id="fix">
The tooltip prop casts controlItem.config.description directly to ReactNode, but according to the BaseControlConfig type, description can be a function that returns ReactNode, just like label. This inconsistency could cause runtime errors if description is a function. It looks like the change handles label functions but missed description.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
tooltip={typeof controlItem.config.description === 'function' ? (controlItem.config.description as Function)() : controlItem.config.description as React.ReactNode}
````
</div>
</details>
</div>
<small><i>Code Review Run #139db3</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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "alexandrusoare (via GitHub)" <gi...@apache.org>.
alexandrusoare commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2939212510
##########
superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.tsx:
##########
@@ -117,43 +117,45 @@ const RawTableView = ({
...props
}: TableViewProps) => {
const tableRef = useRef<HTMLTableElement>(null);
+ const effectivePageSize = initialPageSize ?? DEFAULT_PAGE_SIZE;
+ const [pageIndex, setPageIndex] = useState(initialPageIndex ?? 0);
const initialState = useMemo(
() => ({
- pageSize: initialPageSize ?? DEFAULT_PAGE_SIZE,
- pageIndex: initialPageIndex ?? 0,
+ pageSize: effectivePageSize,
+ pageIndex: 0,
sortBy: initialSortBy,
}),
- [initialPageSize, initialPageIndex, initialSortBy],
+ [effectivePageSize, initialSortBy],
);
const {
getTableProps,
getTableBodyProps,
headerGroups,
- page,
rows,
prepareRow,
- gotoPage,
setSortBy,
- state: { pageIndex, sortBy },
+ state: { sortBy },
} = useTable(
{
columns,
data,
initialState,
- manualPagination: serverPagination,
+ manualPagination: true,
manualSortBy: serverPagination,
Review Comment:
Why did we decide to set TableView to `true` by default?
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/FilterScope.tsx:
##########
@@ -71,7 +71,7 @@ const FilterScope: FC<FilterScopeProps> = ({
);
const updateScopes = useCallback(
- updatedFormValues => {
+ (updatedFormValues: Record<string, any>) => {
Review Comment:
I see that for some we use `string, any` and for some we sue `string, unknown`. What do you think of standardizing this, and use just one of them?
##########
superset-frontend/src/SqlLab/components/QueryTable/index.tsx:
##########
@@ -52,10 +52,10 @@ interface QueryTableQuery extends Omit<
QueryResponse,
'state' | 'sql' | 'progress' | 'results' | 'duration' | 'started'
> {
- state?: Record<string, any>;
- sql?: Record<string, any>;
- progress?: Record<string, any>;
- results?: Record<string, any>;
+ state?: ReactNode;
+ sql?: ReactNode;
+ progress?: ReactNode;
+ results?: ReactNode;
duration?: ReactNode;
started?: ReactNode;
Review Comment:
This seems relevant as well
##########
superset-frontend/src/features/roles/RoleListEditModal.test.tsx:
##########
@@ -145,6 +145,17 @@ describe('RoleListEditModal', () => {
render(<RoleListEditModal {...mockProps} />);
+ // Wait for user hydration to complete before submitting
+ await waitFor(() => {
+ expect(
+ (SupersetClient.get as jest.Mock).mock.calls.some(([call]) =>
+ call.endpoint.includes('/api/v1/security/users/'),
+ ),
+ ).toBe(true);
+ });
+ // Allow the setFieldsValue effect to fire after loading state updates
+ await waitFor(() => Promise.resolve());
Review Comment:
What do you think of this?
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx:
##########
@@ -298,10 +298,12 @@ const AgGridDataTable: FunctionComponent<AgGridTableProps> = memo(
};
const handleColumnHeaderClick = useCallback(
- params => {
+ (params: { column?: { colId?: string; sort?: string } }) => {
const colId = params?.column?.colId;
const sortDir = params?.column?.sort;
- handleColSort(colId, sortDir);
+ if (colId && sortDir) {
+ handleColSort(colId, sortDir);
+ }
Review Comment:
This seems relevant
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r3017196486
##########
superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.tsx:
##########
@@ -117,43 +117,45 @@ const RawTableView = ({
...props
}: TableViewProps) => {
const tableRef = useRef<HTMLTableElement>(null);
+ const effectivePageSize = initialPageSize ?? DEFAULT_PAGE_SIZE;
Review Comment:
Hey, thanks for taking a look. They are here to fix stricter types that come from react 18
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "justinpark (via GitHub)" <gi...@apache.org>.
justinpark commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r3017138474
##########
superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.tsx:
##########
@@ -117,43 +117,45 @@ const RawTableView = ({
...props
}: TableViewProps) => {
const tableRef = useRef<HTMLTableElement>(null);
+ const effectivePageSize = initialPageSize ?? DEFAULT_PAGE_SIZE;
Review Comment:
Personally, I think it would be better to exclude this file's changes, as they are not related to the React 18 upgrade
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2914844197
##########
superset-frontend/src/features/users/UserListModal.tsx:
##########
@@ -133,142 +134,146 @@ function UserListModal({
requiredFields={requiredFields}
initialValues={initialValues}
>
- {(form: FormInstance) => (
- <>
- <FormItem
- name="first_name"
- label={t('First name')}
- rules={[{ required: true, message: t('First name is required') }]}
- >
- <Input
+ {
+ ((form: FormInstance) => (
+ <>
+ <FormItem
name="first_name"
- placeholder={t("Enter the user's first name")}
- />
- </FormItem>
- <FormItem
- name="last_name"
- label={t('Last name')}
- rules={[{ required: true, message: t('Last name is required') }]}
- >
- <Input
+ label={t('First name')}
+ rules={[{ required: true, message: t('First name is required') }]}
+ >
+ <Input
+ name="first_name"
+ placeholder={t("Enter the user's first name")}
+ />
+ </FormItem>
+ <FormItem
name="last_name"
- placeholder={t("Enter the user's last name")}
- />
- </FormItem>
- <FormItem
- name="username"
- label={t('Username')}
- rules={[{ required: true, message: t('Username is required') }]}
- >
- <Input
+ label={t('Last name')}
+ rules={[{ required: true, message: t('Last name is required') }]}
+ >
+ <Input
+ name="last_name"
+ placeholder={t("Enter the user's last name")}
+ />
+ </FormItem>
+ <FormItem
name="username"
- placeholder={t("Enter the user's username")}
- />
- </FormItem>
- <FormItem
- name="active"
- label={t('Is active?')}
- valuePropName="checked"
- >
- <Checkbox
- onChange={checked => {
- form.setFieldsValue({ isActive: checked });
- }}
- />
- </FormItem>
- <FormItem
- name="email"
- label={t('Email')}
- rules={[
- { required: true, message: t('Email is required') },
- {
- type: 'email',
- message: t('Please enter a valid email address'),
- },
- ]}
- >
- <Input name="email" placeholder={t("Enter the user's email")} />
- </FormItem>
- <FormItem
- name="roles"
- label={t('Roles')}
- dependencies={['groups']}
- rules={[atLeastOneRoleOrGroup('groups')]}
- >
- <Select
+ label={t('Username')}
+ rules={[{ required: true, message: t('Username is required') }]}
+ >
+ <Input
+ name="username"
+ placeholder={t("Enter the user's username")}
+ />
+ </FormItem>
+ <FormItem
+ name="active"
+ label={t('Is active?')}
+ valuePropName="checked"
+ >
+ <Checkbox
+ onChange={checked => {
+ form.setFieldsValue({ isActive: checked });
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Form Field Mismatch</b></div>
<div id="fix">
The Checkbox onChange handler sets 'isActive' instead of 'active', preventing the form field from updating correctly when the checkbox is toggled.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
form.setFieldsValue({ active: checked });
````
</div>
</details>
</div>
<small><i>Code Review Run #8f1aed</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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4057440336
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4326835569
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4253262181
- Close button for the 'Connect Database modal doesn't work when there is a closable error popup'
- Layout looks off on this sqllab modal to display sql:
<img width="1692" height="1041" alt="image" src="https://github.com/user-attachments/assets/37509252-3c64-45ac-a9c0-71c1bec7666e" />
- Dashboard/Chart name typing is rerendering on every letter
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4056952791
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4037717009
## **Sequence Diagram**
This PR migrates the collection control from react-sortable-hoc to dnd-kit, changing how list items are reordered while preserving the overall behavior of adding, reordering, and removing items.
```mermaid
sequenceDiagram
participant User
participant CollectionControl
participant DndKit
participant ParentForm
User->>CollectionControl: Drag item to new position
CollectionControl->>DndKit: Wrap items in DndContext and SortableContext
DndKit-->>CollectionControl: DragEnd event with active and over ids
CollectionControl->>CollectionControl: Compute new order with arrayMove
CollectionControl->>ParentForm: onChange(updated item list)
User->>CollectionControl: Click Add item
CollectionControl->>CollectionControl: Create item via itemGenerator
CollectionControl->>ParentForm: onChange(existing items plus new item)
```
---
*Generated by [CodeAnt AI](https://codeant.ai)*
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4058315038
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2934040005
##########
superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.tsx:
##########
@@ -117,43 +117,45 @@ const RawTableView = ({
...props
}: TableViewProps) => {
const tableRef = useRef<HTMLTableElement>(null);
+ const effectivePageSize = initialPageSize ?? DEFAULT_PAGE_SIZE;
+ const [pageIndex, setPageIndex] = useState(initialPageIndex ?? 0);
const initialState = useMemo(
() => ({
- pageSize: initialPageSize ?? DEFAULT_PAGE_SIZE,
- pageIndex: initialPageIndex ?? 0,
+ pageSize: effectivePageSize,
+ pageIndex: 0,
sortBy: initialSortBy,
}),
- [initialPageSize, initialPageIndex, initialSortBy],
+ [effectivePageSize, initialSortBy],
);
const {
getTableProps,
getTableBodyProps,
headerGroups,
- page,
rows,
prepareRow,
- gotoPage,
setSortBy,
- state: { pageIndex, sortBy },
+ state: { sortBy },
} = useTable(
{
columns,
data,
initialState,
- manualPagination: serverPagination,
+ manualPagination: true,
manualSortBy: serverPagination,
- pageCount: serverPagination
- ? Math.ceil(totalCount / initialState.pageSize)
- : undefined,
autoResetSortBy: false,
},
useFilters,
useSortBy,
- ...(withPagination ? [usePagination] : []),
);
+ const content = useMemo(() => {
+ if (!withPagination) return rows;
+ const start = pageIndex * effectivePageSize;
+ return rows.slice(start, start + effectivePageSize);
+ }, [withPagination, rows, pageIndex, effectivePageSize]);
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Server pagination data slicing error</b></div>
<div id="fix">
For server-side pagination, the data prop should contain only the current page's data from the server, so client-side slicing in the content useMemo is incorrect and can display wrong or empty rows. The condition should return rows un sliced when serverPagination is enabled.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
const content = useMemo(() => {
if (!withPagination || serverPagination) return rows;
const start = pageIndex * effectivePageSize;
return rows.slice(start, start + effectivePageSize);
}, [withPagination, rows, pageIndex, effectivePageSize, serverPagination]);
````
</div>
</details>
</div>
<small><i>Code Review Run #098e84</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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4252826065
๐ช [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/24458659002) for [156af9a](https://github.com/apache/superset/commit/156af9a)
โข **Environment:** http://52.33.197.214:8080 (admin/admin)
โข **Lifetime:** 48h auto-cleanup
โข **Updates:** New commits create fresh environments automatically
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r3017196486
##########
superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.tsx:
##########
@@ -117,43 +117,45 @@ const RawTableView = ({
...props
}: TableViewProps) => {
const tableRef = useRef<HTMLTableElement>(null);
+ const effectivePageSize = initialPageSize ?? DEFAULT_PAGE_SIZE;
Review Comment:
Hey, thanks for taking a look. They are here to fix stricter types that come from react 19
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Msyavuz/chore/react 18 clean [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4033016804
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4038818373
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2924138550
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx:
##########
@@ -146,15 +147,21 @@ describe('SaveDatasetModal', () => {
// Click the overwrite select component
const select = screen.getByRole('combobox', { name: /existing dataset/i })!;
- userEvent.click(select);
+ await userEvent.click(select);
- await waitFor(() =>
- expect(screen.queryByText('Loading...')).not.toBeVisible(),
- );
+ // Advance timers to flush debounced fetches in AsyncSelect
+ await act(async () => {
+ jest.runAllTimers();
+ });
+
+ await waitFor(() => {
+ const loading = screen.queryByText('Loading...');
+ expect(loading === null || !loading.checkVisibility()).toBe(true);
+ });
Review Comment:
**Suggestion:** The assertion uses `loading.checkVisibility()`, but `checkVisibility` is not reliably implemented in Jest/JSDOM environments and can throw a runtime `TypeError` when the loading element exists. Use a Testing Library assertion that checks removal from the DOM instead of calling browser-only visibility APIs. [type error]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ ๏ธ SaveDatasetModal overwrite tests can fail in CI.
- โ ๏ธ React 18 migration test suite becomes flaky.
- โ PR validation may fail on frontend pipeline.
```
</details>
```suggestion
await waitFor(() => {
expect(screen.queryByText('Loading...')).not.toBeInTheDocument();
});
```
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Run frontend Jest tests that include
`superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx`
(matched by `superset-frontend/jest.config.js:23` testRegex).
2. Jest uses custom JSDOM environment from `superset-frontend/jest.config.js:38` and
`spec/helpers/jsDomWithFetchAPI.ts:20-43`; no `checkVisibility` polyfill is added in setup
(`spec/helpers/setup.ts:19-40`).
3. In tests `renders the overwrite button...` and `renders a confirm overwrite screen...`,
the callback at `SaveDatasetModal.test.tsx:157-160` (also `190-192`) evaluates
`loading.checkVisibility()` when `screen.queryByText('Loading...')` returns an element.
4. When that branch executes, JSDOM can throw `TypeError: loading.checkVisibility is not a
function`, failing/flaking these overwrite-flow tests.
```
</details>
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx
**Line:** 157:160
**Comment:**
*Type Error: The assertion uses `loading.checkVisibility()`, but `checkVisibility` is not reliably implemented in Jest/JSDOM environments and can throw a runtime `TypeError` when the loading element exists. Use a Testing Library assertion that checks removal from the DOM instead of calling browser-only visibility APIs.
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>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38563&comment_hash=7e58dbae65c27b6ee5489273e344a23cb66a4f755f3c8bce3c4fce4879f89bb8&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38563&comment_hash=7e58dbae65c27b6ee5489273e344a23cb66a4f755f3c8bce3c4fce4879f89bb8&reaction=dislike'>๐</a>
##########
superset-frontend/src/features/roles/RoleListEditModal.test.tsx:
##########
@@ -145,6 +145,17 @@ describe('RoleListEditModal', () => {
render(<RoleListEditModal {...mockProps} />);
+ // Wait for user hydration to complete before submitting
+ await waitFor(() => {
+ expect(
+ (SupersetClient.get as jest.Mock).mock.calls.some(([call]) =>
+ call.endpoint.includes('/api/v1/security/users/'),
+ ),
+ ).toBe(true);
+ });
+ // Allow the setFieldsValue effect to fire after loading state updates
+ await waitFor(() => Promise.resolve());
Review Comment:
**Suggestion:** The added `waitFor(() => Promise.resolve())` is a no-op and does not actually wait for hydration to finish, so the test can still submit before `setFieldsValue` updates `roleUsers`, causing flaky failures. Replace it with a real observable assertion that confirms hydrated user values are rendered before clicking save. [logic error]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ ๏ธ Flaky role edit submission test in CI.
- โ ๏ธ React 18 timing changes amplify race windows.
- โ User-ID mapping assertion intermittently fails.
```
</details>
```suggestion
await waitFor(() => {
expect(screen.getByText('johndoe')).toBeInTheDocument();
});
```
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Run `RoleListEditModal` test `submit maps {value,label} form values to numeric ID
arrays` in `superset-frontend/src/features/roles/RoleListEditModal.test.tsx` (render at
line 146, submit click at line 163).
2. The test only waits for `SupersetClient.get` call presence (lines 149-155), then
executes `await waitFor(() => Promise.resolve())` (line 157), which does not assert any
hydration-visible condition.
3. In `superset-frontend/src/features/roles/RoleListEditModal.tsx`, user form hydration
happens later in `useEffect` at lines 215-226 via `formRef.current.setFieldsValue({
roleUsers: ... })`, after `loadingRoleUsers` flips false in `fetchPaginatedData` finally
block (`superset-frontend/src/utils/fetchOptions.ts:103-112`).
4. If save is clicked before that effect commits, `handleFormSubmit`
(`RoleListEditModal.tsx:280-289`) reads `values.roleUsers` as empty and calls
`updateRoleUsers(id, [])`, causing intermittent failure of expected `[5, 7]` assertion in
the test.
```
</details>
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/features/roles/RoleListEditModal.test.tsx
**Line:** 156:157
**Comment:**
*Logic Error: The added `waitFor(() => Promise.resolve())` is a no-op and does not actually wait for hydration to finish, so the test can still submit before `setFieldsValue` updates `roleUsers`, causing flaky failures. Replace it with a real observable assertion that confirms hydrated user values are rendered before clicking save.
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>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38563&comment_hash=1036dca4dc7647b6d5fb9557f2a81b318e85d845fc45c3cae4c5765303150eb8&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38563&comment_hash=1036dca4dc7647b6d5fb9557f2a81b318e85d845fc45c3cae4c5765303150eb8&reaction=dislike'>๐</a>
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz merged PR #38563:
URL: https://github.com/apache/superset/pull/38563
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4243405272
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/38563?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `64.13043%` with `66 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 62.93%. Comparing base ([`c2a35e2`](https://app.codecov.io/gh/apache/superset/commit/c2a35e2eeae04384ebfb70d663268f14c299da75?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`fc6d783`](https://app.codecov.io/gh/apache/superset/commit/fc6d7835785457815ba1aad1e9282cc63265846f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/38563?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...re/components/controls/CollectionControl/index.tsx](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Fexplore%2Fcomponents%2Fcontrols%2FCollectionControl%2Findex.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xsZWN0aW9uQ29udHJvbC9pbmRleC50c3g=) | 67.30% | [17 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...rset-frontend/src/features/users/UserListModal.tsx](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Ffeatures%2Fusers%2FUserListModal.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVzL3VzZXJzL1VzZXJMaXN0TW9kYWwudHN4) | 28.57% | [10 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...perset-frontend/src/components/GridTable/index.tsx](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Fcomponents%2FGridTable%2Findex.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvR3JpZFRhYmxlL2luZGV4LnRzeA==) | 20.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ugin-chart-ag-grid-table/src/AgGridTable/index.tsx](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&filepath=superset-frontend%2Fplugins%2Fplugin-chart-ag-grid-table%2Fsrc%2FAgGridTable%2Findex.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtYWctZ3JpZC10YWJsZS9zcmMvQWdHcmlkVGFibGUvaW5kZXgudHN4) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...n-chart-cartodiagram/src/components/ChartLayer.tsx](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&filepath=superset-frontend%2Fplugins%2Fplugin-chart-cartodiagram%2Fsrc%2Fcomponents%2FChartLayer.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtY2FydG9kaWFncmFtL3NyYy9jb21wb25lbnRzL0NoYXJ0TGF5ZXIudHN4) | 40.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset-frontend/src/views/menu.tsx](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Fviews%2Fmenu.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL21lbnUudHN4) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...set-ui-core/src/components/TruncatedList/index.tsx](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&filepath=superset-frontend%2Fpackages%2Fsuperset-ui-core%2Fsrc%2Fcomponents%2FTruncatedList%2Findex.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvY29tcG9uZW50cy9UcnVuY2F0ZWRMaXN0L2luZGV4LnRzeA==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...lugin-chart-echarts/src/Treemap/EchartsTreemap.tsx](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&filepath=superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Fsrc%2FTreemap%2FEchartsTreemap.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVHJlZW1hcC9FY2hhcnRzVHJlZW1hcC50c3g=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset-frontend/src/components/Chart/Chart.tsx](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Fcomponents%2FChart%2FChart.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2hhcnQvQ2hhcnQudHN4) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ilters/FilterBar/FilterControls/FilterControls.tsx](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Fdashboard%2Fcomponents%2FnativeFilters%2FFilterBar%2FFilterControls%2FFilterControls.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyQmFyL0ZpbHRlckNvbnRyb2xzL0ZpbHRlckNvbnRyb2xzLnRzeA==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| ... and [13 more](https://app.codecov.io/gh/apache/superset/pull/38563?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #38563 +/- ##
==========================================
- Coverage 64.45% 62.93% -1.52%
==========================================
Files 2555 2538 -17
Lines 132712 131110 -1602
Branches 30799 30090 -709
==========================================
- Hits 85534 82516 -3018
- Misses 45692 47090 +1398
- Partials 1486 1504 +18
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/38563/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage ฮ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/38563/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.96% <รธ> (รธ)` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/38563/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/38563/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/38563/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.75% <รธ> (รธ)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/38563/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `60.44% <รธ> (-1.83%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/38563/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `60.31% <รธ> (รธ)` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/38563/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/38563?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4038427224
## **Sequence Diagram**
This PR rewires the Explore collection control to use dnd kit for drag and drop, changing how item reordering events flow from the UI back to the parent form while keeping the visible behavior the same.
```mermaid
sequenceDiagram
participant User
participant CollectionControl
participant DragDropEngine
participant ParentForm
User->>CollectionControl: Drag item handle
CollectionControl->>DragDropEngine: Register drag sensors and item ids
DragDropEngine-->>CollectionControl: Drag end event (active id, over id)
CollectionControl->>CollectionControl: Compute new order with arrayMove
CollectionControl->>ParentForm: onChange(updated collection)
ParentForm-->>CollectionControl: Rerender with new item order
```
---
*Generated by [CodeAnt AI](https://codeant.ai)*
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Msyavuz/chore/react 18 clean [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4033026927
## **Sequence Diagram**
This PR reimplements the Explore CollectionControl drag-and-drop using dnd-kit, so reordering items now flows through a DnD context that updates the collection value via arrayMove and calls the parent onChange handler.
```mermaid
sequenceDiagram
participant User
participant CollectionControl
participant DnD
participant ParentForm
User->>CollectionControl: Drag collection item handle
CollectionControl->>DnD: Start drag session with item id
User->>DnD: Drop item at new position
DnD-->>CollectionControl: Drag end with active and over ids
CollectionControl->>CollectionControl: Compute new order with arrayMove
CollectionControl->>ParentForm: onChange(updated collection items)
ParentForm-->>CollectionControl: Rerender with new item order
```
---
*Generated by [CodeAnt AI](https://codeant.ai)*
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4046194611
### <span aria-hidden="true">โ
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">๐จ</span> Latest commit | 2479f0fc187e40de81b391cf691ab54d0985fe51 |
|<span aria-hidden="true">๐</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/69b2a9e0803b5000074fef7e |
|<span aria-hidden="true">๐</span> Deploy Preview | [https://deploy-preview-38563--superset-docs-preview.netlify.app](https://deploy-preview-38563--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">๐ฑ</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-38563--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4047649446
## **Sequence Diagram**
This PR migrates Superset frontend rendering to React 18 roots and replaces legacy list sorting with dnd kit. The diagram shows the core runtime flow for app mount, item reordering, and dynamic chart root lifecycle updates.
```mermaid
sequenceDiagram
participant User
participant Frontend
participant DndKit
participant ChartLayer
participant React18Root
User->>Frontend: Open application
Frontend->>React18Root: Create root and render app
User->>Frontend: Drag collection item
Frontend->>DndKit: Handle drag end and compute new order
DndKit-->>Frontend: Return reordered items
Frontend-->>Frontend: Save updated control value
Frontend->>ChartLayer: Refresh dynamic chart layer
ChartLayer->>React18Root: Render updated charts and unmount removed charts
```
---
*Generated by [CodeAnt AI](https://codeant.ai)*
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4048780711
## **Sequence Diagram**
This PR migrates frontend mounting to React 18 roots and updates chart overlay rendering to use root-based mount and unmount behavior. It also replaces legacy sortable logic with dnd kit so collection items reorder through a drag handle and emit updated order.
```mermaid
sequenceDiagram
participant Browser
participant FrontendBootstrap
participant ReactRoot
participant App
participant User
participant CollectionControl
participant DndKit
participant ChartLayer
Browser->>FrontendBootstrap: Load frontend entry
FrontendBootstrap->>ReactRoot: Create root at mount point
ReactRoot->>App: Render app tree
User->>CollectionControl: Drag item by handle
CollectionControl->>DndKit: Track drag and compute new order
DndKit-->>CollectionControl: Return reordered item ids
CollectionControl-->>App: Emit updated collection value
App->>ChartLayer: Render or update map chart markers
ChartLayer->>ReactRoot: Render chart roots and unmount removed roots
```
---
*Generated by [CodeAnt AI](https://codeant.ai)*
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "alexandrusoare (via GitHub)" <gi...@apache.org>.
alexandrusoare commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4253341852
- While on database connections on page, when clicking on sync, in the toast the icon is misaligned with the text
- Weird behaviour for dashboard name edit as sometimes you can delete or add any letters to it
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4038244421
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2917597286
##########
superset-frontend/src/features/reports/ReportModal/index.tsx:
##########
@@ -366,7 +367,7 @@ function ReportModal({
}}
onError={setCronError}
/>
- <StyledCronError>{cronError}</StyledCronError>
+ <StyledCronError>{cronError as ReactNode}</StyledCronError>
<div
Review Comment:
<!-- Bito Reply -->
The suggestion modifies the error display to show `cronError?.description` instead of casting `cronError` to `ReactNode`. This assumes `cronError` is an object with a `description` property, potentially improving readability by displaying a specific error message rather than the raw error object. However, if `cronError` lacks a `description` field or is already a string, this might not enhance the code and could lead to undefined display.
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2917628114
##########
superset-frontend/packages/superset-ui-core/src/components/TableCollection/utils.tsx:
##########
@@ -91,7 +91,7 @@ export function mapColumns<T extends object>(
return columns.map(column => {
const { isSorted, isSortedDesc } = getSortingInfo(headerGroups, column.id);
return {
- title: column.Header,
+ title: column.Header as ReactNode,
Review Comment:
**Suggestion:** The `title` field passed to the Ant Design table columns uses `column.Header` directly, but in `react-table` this can be a render function rather than a React node; passing the function itself as a React child will cause React runtime errors ("Functions are not valid as a React child") instead of rendering the intended header label, so this should use `column.render('Header')` to correctly support both string/ReactNode and functional headers. [logic error]
<details>
<summary><b>Severity Level:</b> Critical ๐จ</summary>
```mdx
- โ Bulk-select header crashes in ListView table mode.
- โ TableCollection breaks with any functional react-table Header.
- โ ๏ธ Users lose select-all checkbox on affected list views.
```
</details>
```suggestion
title: column.render('Header') as ReactNode,
```
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Note in `src/components/ListView/ListView.tsx` lines 145โ158 that
`bulkSelectColumnConfig` defines a `Header` as a render function returning a `<Checkbox>`:
`Header: ({ getToggleAllRowsSelectedProps }: any) => (<Checkbox ... />)`.
2. In `src/components/ListView/utils.ts` lines 188โ205 and 249โ275, `useListViewState`
receives `bulkSelectColumnConfig` and, when `bulkSelectMode` is truthy, builds
`columnsWithSelect` by prepending this config to the regular `columns` array and passes it
into `useTable` from `react-table`, producing `headerGroups` and `columns` whose `Header`
for the bulk-select column is still that function.
3. In `src/components/ListView/ListView.tsx` lines 269โ323 and 492โ526, `ListView` calls
`useListViewState` and, when `viewMode === 'table'`, renders `<TableCollection ...>`
passing `headerGroups`, `rows`, and `columns` from the react-table instance, so the
bulk-select column with functional `Header` flows into `TableCollection`.
4. In `packages/superset-ui-core/src/components/TableCollection/index.tsx` lines 159โ183
and 289โ323, `TableCollection` calls `mapColumns(columns, headerGroups,
columnsForWrapText)` from `./utils` and feeds the result as `columns` into the Ant Design
`<Table>` wrapper; in `utils.tsx` lines 86โ105 and specifically line 94, `mapColumns` sets
`title: column.Header as ReactNode`, so for the bulk-select column `title` becomes a
function, which React 18 will try to render as a child in a `<th>` and trigger the runtime
error "Functions are not valid as a React child" instead of rendering the checkbox header.
```
</details>
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/packages/superset-ui-core/src/components/TableCollection/utils.tsx
**Line:** 94:94
**Comment:**
*Logic Error: The `title` field passed to the Ant Design table columns uses `column.Header` directly, but in `react-table` this can be a render function rather than a React node; passing the function itself as a React child will cause React runtime errors ("Functions are not valid as a React child") instead of rendering the intended header label, so this should use `column.render('Header')` to correctly support both string/ReactNode and functional headers.
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>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38563&comment_hash=1ce55649f111df2a3b55989a1880d005f27553807d0d08c1f9f7c316ccaa383b&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38563&comment_hash=1ce55649f111df2a3b55989a1880d005f27553807d0d08c1f9f7c316ccaa383b&reaction=dislike'>๐</a>
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4258679650
- If I select the dark theme from the navigation menu and then assign a different theme to the dark theme system, I need to reload the page to see the changes applied.
- In the Edit CSS template properties modal I didn't see the css input field, there is only the label.
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4048169443
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r3167621904
##########
superset-frontend/src/components/Chart/Chart.tsx:
##########
@@ -235,16 +235,13 @@ class Chart extends PureComponent<ChartProps, {}> {
);
}
- handleRenderContainerFailure(
- error: Error,
- info: { componentStack: string } | null,
- ) {
+ handleRenderContainerFailure(error: Error, info: ErrorInfo) {
Review Comment:
renderStartTime is declared as `any ` (not in this PR) and never assigned anywhere in this file, so when `ErrorBoundary `triggers `handleRenderContainerFailure` we dispatch start_offset: undefined and duration: NaN to LOG_ACTIONS_RENDER_CHART, which also violates the number contract declared on Actions.logEvent. ChartRenderer and Markdown already handle this correctly (typed as number, initialized in the constructor, refreshed before render).
Suggest doing the same here: type as number and set this.renderStartTime = Logger.getTimestamp() in the constructor, so the offset is always finite even if the boundary fires before componentDidMount.
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx:
##########
@@ -220,11 +222,16 @@ export default function getControlItemsMap({
}}
>
<>
- {controlItem.config.label}
+ {typeof controlItem.config.label === 'function'
+ ? (controlItem.config.label as Function)()
+ : controlItem.config.label}
+
{controlItem.config.description && (
<InfoTooltip
placement="top"
- tooltip={controlItem.config.description}
+ tooltip={
+ controlItem.config.description as React.ReactNode
Review Comment:
Same gotcha as `label `just above; `description `can also be `(state, controlState, chartState?) => ReactNode` (see `BaseControlConfig `in superset-ui-chart-controls/src/types.ts).
Casting straight to `ReactNode `would let a function slip through and render as [object Function] (or trip React 18's "Functions are not valid as a React child"). Could we mirror the `typeof === 'function'` guard you already added for label?
Something like:
```
tooltip={
typeof controlItem.config.description === 'function'
? (controlItem.config.description as Function)()
: controlItem.config.description
}
```
##########
superset-frontend/src/SqlLab/components/QueryTable/index.tsx:
##########
@@ -265,7 +274,7 @@ const QueryTable = ({
buttonStyle="link"
onClick={() => onUserClicked(q.userId)}
>
- {q.user}
+ {q.user as ReactNode}
Review Comment:
Is this already typed?
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "alexandrusoare (via GitHub)" <gi...@apache.org>.
alexandrusoare commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r3168776406
##########
superset-frontend/src/features/roles/RoleListEditModal.tsx:
##########
@@ -330,45 +330,47 @@ function RoleListEditModal({
initialValues={initialValues}
requiredFields={['roleName']}
>
- {(form: FormInstance) => {
- formRef.current = form;
+ {
+ ((form: FormInstance) => {
+ formRef.current = form;
- return (
- <Tabs
- activeKey={activeTabKey}
- onChange={activeKey => setActiveTabKey(activeKey)}
- >
- <Tabs.TabPane
- tab={roleTabs.edit.name}
- key={roleTabs.edit.key}
- forceRender
+ return (
+ <Tabs
+ activeKey={activeTabKey}
+ onChange={activeKey => setActiveTabKey(activeKey)}
>
- <>
- <RoleNameField />
- <PermissionsField
- addDangerToast={addDangerToast}
- loading={loadingRolePermissions}
+ <Tabs.TabPane
+ tab={roleTabs.edit.name}
+ key={roleTabs.edit.key}
+ forceRender
+ >
+ <>
+ <RoleNameField />
+ <PermissionsField
+ addDangerToast={addDangerToast}
+ loading={loadingRolePermissions}
+ />
+ <UsersField
+ addDangerToast={addDangerToast}
+ loading={loadingRoleUsers}
+ />
+ <GroupsField
+ addDangerToast={addDangerToast}
+ loading={loadingRoleGroups}
+ />
+ </>
+ </Tabs.TabPane>
+ <Tabs.TabPane tab={roleTabs.users.name} key={roleTabs.users.key}>
+ <TableView
+ columns={userColumns}
+ data={roleUsers}
+ emptyWrapperType={EmptyWrapperType.Small}
/>
- <UsersField
- addDangerToast={addDangerToast}
- loading={loadingRoleUsers}
- />
- <GroupsField
- addDangerToast={addDangerToast}
- loading={loadingRoleGroups}
- />
- </>
- </Tabs.TabPane>
- <Tabs.TabPane tab={roleTabs.users.name} key={roleTabs.users.key}>
- <TableView
- columns={userColumns}
- data={roleUsers}
- emptyWrapperType={EmptyWrapperType.Small}
- />
- </Tabs.TabPane>
- </Tabs>
- );
- }}
+ </Tabs.TabPane>
+ </Tabs>
+ );
+ }) as unknown as ReactNode
Review Comment:
?
##########
superset-frontend/src/features/reports/ReportModal/index.tsx:
##########
@@ -366,7 +367,7 @@ function ReportModal({
}}
onError={setCronError}
/>
- <StyledCronError>{cronError}</StyledCronError>
+ <StyledCronError>{cronError as ReactNode}</StyledCronError>
<div
Review Comment:
Not sure if you've seen this
##########
superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.tsx:
##########
@@ -117,43 +117,45 @@ const RawTableView = ({
...props
}: TableViewProps) => {
const tableRef = useRef<HTMLTableElement>(null);
+ const effectivePageSize = initialPageSize ?? DEFAULT_PAGE_SIZE;
+ const [pageIndex, setPageIndex] = useState(initialPageIndex ?? 0);
const initialState = useMemo(
() => ({
- pageSize: initialPageSize ?? DEFAULT_PAGE_SIZE,
- pageIndex: initialPageIndex ?? 0,
+ pageSize: effectivePageSize,
+ pageIndex: 0,
sortBy: initialSortBy,
}),
- [initialPageSize, initialPageIndex, initialSortBy],
+ [effectivePageSize, initialSortBy],
);
const {
getTableProps,
getTableBodyProps,
headerGroups,
- page,
rows,
prepareRow,
- gotoPage,
setSortBy,
- state: { pageIndex, sortBy },
+ state: { sortBy },
} = useTable(
{
columns,
data,
initialState,
- manualPagination: serverPagination,
+ manualPagination: true,
manualSortBy: serverPagination,
Review Comment:
?
##########
superset-frontend/packages/superset-ui-core/src/components/TableCollection/utils.tsx:
##########
@@ -91,7 +91,7 @@ export function mapColumns<T extends object>(
return columns.map(column => {
const { isSorted, isSortedDesc } = getSortingInfo(headerGroups, column.id);
return {
- title: column.Header,
+ title: column.Header as ReactNode,
Review Comment:
Not sure if you've seen this, looks relevant?
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2932985607
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx:
##########
@@ -298,10 +298,12 @@ const AgGridDataTable: FunctionComponent<AgGridTableProps> = memo(
};
const handleColumnHeaderClick = useCallback(
- params => {
+ (params: { column?: { colId?: string; sort?: string } }) => {
const colId = params?.column?.colId;
const sortDir = params?.column?.sort;
- handleColSort(colId, sortDir);
+ if (colId && sortDir) {
+ handleColSort(colId, sortDir);
+ }
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect sort clearing logic</b></div>
<div id="fix">
The conditional `if (colId && sortDir)` incorrectly skips calling `handleColSort` when `sortDir` is `null`, but `handleColSort` handles `sortDir == null` to clear sorting. Since column header clicks can set `sortDir` to `null` for unsorting, the check should only verify `colId` is defined.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
if (colId) {
handleColSort(colId, sortDir);
}
````
</div>
</details>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a href="https://github.com/apache/superset/blob/28ae7f6/.cursor/rules/dev-standard.mdc#L16">dev-standard.mdc:16</a>
</li>
<li>
Rule Violated: <a href="https://github.com/apache/superset/blob/28ae7f6/AGENTS.md#L78">AGENTS.md:78</a>
</li>
</ul>
</details>
<small><i>Code Review Run #d1e84f</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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4034419821
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2918081471
##########
superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx:
##########
@@ -440,7 +440,7 @@ describe('Table view with pagination', () => {
expect(nextPageButton).toBeInTheDocument();
// Click next page
- userEvent.click(nextPageButton);
+ await userEvent.click(nextPageButton);
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Inconsistent async userEvent.click usage</b></div>
<div id="fix">
The change correctly adds 'await' to userEvent.click, as this method is asynchronous in @testing-library/user-event v12.8.3. However, 13 other userEvent.click calls in this file lack 'await', which can cause inconsistent test behavior and flakiness. All userEvent.click invocations should be awaited to ensure events are fully processed before assertions.
</div>
</div>
<small><i>Code Review Run #5f870a</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/AsyncEsmComponent/index.tsx:
##########
@@ -91,15 +87,14 @@ export function AsyncEsmComponent<
return promise;
}
- type AsyncComponent = ForwardRefExoticComponent<
- PropsWithoutRef<FullProps> & RefAttributes<ComponentType<FullProps>>
- > & {
+ type AsyncComponent = ReturnType<typeof forwardRef> & {
preload?: typeof waitForPromise;
};
+ // @ts-expect-error -- generic forwardRef has PropsWithoutRef incompatibility with FullProps
const AsyncComponent: AsyncComponent = forwardRef(function AsyncComponent(
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Type alias redeclaration causes linter error</b></div>
<div id="fix">
The `AsyncComponent` type is declared twice: once as a type alias (line 90) and again as a const variable (line 95). Remove the type alias declaration and apply the type annotation directly to the const declaration to resolve the redeclaration error.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
// @ts-expect-error -- generic forwardRef has PropsWithoutRef incompatibility with FullProps
const AsyncComponent: ReturnType<typeof forwardRef> & { preload?: typeof waitForPromise } = forwardRef(function AsyncComponent(
````
</div>
</details>
</div>
<small><i>Code Review Run #5f870a</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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4322097861
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4328410084
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4252542070
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4245445469
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4046170206
## **Sequence Diagram**
This PR's core runtime changes move frontend mounting to React 18 root rendering and replace collection drag sorting with dnd kit based reordering. The diagram highlights how the app now boots with create root and how drag actions produce stable ordered updates.
```mermaid
sequenceDiagram
participant User
participant EntryPoint
participant ReactRoot
participant CollectionControl
participant DndKit
participant AppState
User->>EntryPoint: Open frontend page
EntryPoint->>ReactRoot: Create root at mount element
ReactRoot->>ReactRoot: Render app providers and screens
User->>CollectionControl: Drag list item to new position
CollectionControl->>DndKit: Process drag end and target position
DndKit-->>CollectionControl: Return reordered item ids
CollectionControl->>AppState: Apply array move and emit onChange
AppState-->>User: Render updated item order
```
---
*Generated by [CodeAnt AI](https://codeant.ai)*
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Msyavuz/chore/react 18 clean [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2913232545
##########
superset-frontend/src/SqlLab/components/QueryTable/index.tsx:
##########
@@ -52,10 +52,10 @@ interface QueryTableQuery extends Omit<
QueryResponse,
'state' | 'sql' | 'progress' | 'results' | 'duration' | 'started'
> {
- state?: Record<string, any>;
- sql?: Record<string, any>;
- progress?: Record<string, any>;
- results?: Record<string, any>;
+ state?: ReactNode;
+ sql?: ReactNode;
+ progress?: ReactNode;
+ results?: ReactNode;
duration?: ReactNode;
started?: ReactNode;
Review Comment:
**Suggestion:** The `QueryTableQuery` type omits and redefines several fields from `QueryResponse` to be `ReactNode`, but `user` and `db` are not omitted and thus retain their original `Record<string, any>` types while later being assigned JSX elements, creating a type mismatch that breaks the contract of `QueryTableQuery` and can cause TypeScript errors or misuse of these fields as non-object values elsewhere. [type error]
<details>
<summary><b>Severity Level:</b> Critical ๐จ</summary>
```mdx
- โ SqlLab QueryTable file fails TypeScript type-checking.
- โ Frontend build/tests can fail, blocking deployments.
- โ ๏ธ Query history UI cannot be shipped with current types.
```
</details>
```suggestion
'state' | 'sql' | 'progress' | 'results' | 'duration' | 'started' | 'user' | 'db'
> {
state?: ReactNode;
sql?: ReactNode;
progress?: ReactNode;
results?: ReactNode;
duration?: ReactNode;
started?: ReactNode;
user?: ReactNode;
db?: ReactNode;
```
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Open `superset-frontend/src/SqlLab/components/QueryTable/index.tsx` and locate
`QueryTableQuery` at lines 51โ61, which extends `QueryResponse` but only omits `'state' |
'sql' | 'progress' | 'results' | 'duration' | 'started'`.
2. In the same file, inspect the `data` `useMemo` mapping at lines 250โ279 where `const q
= rest as QueryTableQuery;` is created and `q.user` and `q.db` are assigned JSX `<Button>`
elements (lines 262โ279).
3. Inspect `superset-frontend/src/SqlLab/fixtures.ts` where `baseQuery: QueryResponse` is
defined at lines 587โ672 and note that `db` and `user` are initialized as objects: `db: {
key: 'main' }` and `user: { key: 'admin' }` at lines 611โ612, evidencing that
`QueryResponse['db']` and `QueryResponse['user']` are object-shaped, not ReactNode.
4. Run the TypeScript type checker or frontend build (e.g., `npm test` or `npm run build`)
in this PR: the compiler will type-check `QueryTable/index.tsx`, see `q.user` and `q.db`
being assigned React elements where their inherited types from `QueryResponse` are object
types, and emit errors similar to "Type 'ReactElement<...>' is not assignable to type
'Record<string, any>'" for these assignments, causing the SqlLab frontend to fail
compilation.
```
</details>
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/SqlLab/components/QueryTable/index.tsx
**Line:** 53:60
**Comment:**
*Type Error: The `QueryTableQuery` type omits and redefines several fields from `QueryResponse` to be `ReactNode`, but `user` and `db` are not omitted and thus retain their original `Record<string, any>` types while later being assigned JSX elements, creating a type mismatch that breaks the contract of `QueryTableQuery` and can cause TypeScript errors or misuse of these fields as non-object values elsewhere.
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>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38563&comment_hash=8d5f297e6c81f47c3ab3af656f9579f1c4660b1a65d2c57113b883a59b633056&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38563&comment_hash=8d5f297e6c81f47c3ab3af656f9579f1c4660b1a65d2c57113b883a59b633056&reaction=dislike'>๐</a>
##########
superset-frontend/src/components/Chart/Chart.tsx:
##########
@@ -235,16 +235,13 @@ class Chart extends PureComponent<ChartProps, {}> {
);
}
- handleRenderContainerFailure(
- error: Error,
- info: { componentStack: string } | null,
- ) {
+ handleRenderContainerFailure(error: Error, info: ErrorInfo) {
Review Comment:
**Suggestion:** The error handler logs chart render metrics using `this.renderStartTime`, but this field is never initialized in the `Chart` component, so `start_offset` will be `undefined` and `duration` will be `NaN`, producing invalid telemetry data whenever the error boundary is triggered. Use a safe fallback when `renderStartTime` is not a valid number so that logging always uses numeric values. [logic error]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ ๏ธ Chart error telemetry has undefined start_offset and NaN duration.
- โ ๏ธ LOG_ACTIONS_RENDER_CHART analytics receive corrupted timing data.
```
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2917042125
##########
superset-frontend/src/pages/SqlLab/LocationContext.tsx:
##########
@@ -31,7 +31,9 @@ const { Provider } = locationContext;
const EMPTY_STATE: LocationState = {};
-export const LocationProvider: FC = ({ children }: { children: ReactNode }) => {
+export const LocationProvider: FC<{ children?: ReactNode }> = ({
+ children,
+}) => {
const location = useLocation<LocationState>();
if (location.state) {
return <Provider value={location.state}>{children}</Provider>;
Review Comment:
**Suggestion:** The context value can lack a `requestedQuery` object (e.g., when `location.state` is present without that field or when falling back to `EMPTY_STATE`), but `PopEditorTab` unconditionally spreads `requestedQuery` into a new object; if it is `undefined`, this will throw a runtime `TypeError: Cannot convert undefined or null to object`. Normalizing the context so that `requestedQuery` is always at least an empty object prevents this crash. [logic error]
<details>
<summary><b>Severity Level:</b> Critical ๐จ</summary>
```mdx
- โ Visiting SqlLab without query state crashes PopEditorTab.
- โ Direct navigation to '/sqllab' can break SQL Lab.
- โ ๏ธ Potential untested paths crash future SqlLab unit tests.
```
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "alexandrusoare (via GitHub)" <gi...@apache.org>.
alexandrusoare commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2917513249
##########
superset-frontend/src/components/Chart/DrillDetail/DrillDetailTableControls.tsx:
##########
@@ -109,7 +109,7 @@ export default function TableControls({
>
{colName}
</span>
- <strong data-test="filter-val">{val}</strong>
+ <strong data-test="filter-val">{String(val)}</strong>
Review Comment:
Why are we stringifying this?
##########
superset-frontend/packages/superset-ui-core/src/components/Button/types.ts:
##########
@@ -49,5 +48,5 @@ export type ButtonProps = Omit<AntdButtonProps, 'css'> & {
buttonStyle?: ButtonStyle;
cta?: boolean;
showMarginRight?: boolean;
- icon?: IconType;
+ icon?: ReactNode;
Review Comment:
What is the benefit of using ReactNode instead of the IconType?
##########
superset-frontend/src/features/reports/ReportModal/index.tsx:
##########
@@ -366,7 +367,7 @@ function ReportModal({
}}
onError={setCronError}
/>
- <StyledCronError>{cronError}</StyledCronError>
+ <StyledCronError>{cronError as ReactNode}</StyledCronError>
<div
Review Comment:
Is this relevant?
##########
superset-frontend/src/features/roles/RoleListEditModal.tsx:
##########
@@ -330,45 +330,47 @@ function RoleListEditModal({
initialValues={initialValues}
requiredFields={['roleName']}
>
- {(form: FormInstance) => {
- formRef.current = form;
+ {
+ ((form: FormInstance) => {
+ formRef.current = form;
- return (
- <Tabs
- activeKey={activeTabKey}
- onChange={activeKey => setActiveTabKey(activeKey)}
- >
- <Tabs.TabPane
- tab={roleTabs.edit.name}
- key={roleTabs.edit.key}
- forceRender
+ return (
+ <Tabs
+ activeKey={activeTabKey}
+ onChange={activeKey => setActiveTabKey(activeKey)}
>
- <>
- <RoleNameField />
- <PermissionsField
- addDangerToast={addDangerToast}
- loading={loadingRolePermissions}
+ <Tabs.TabPane
+ tab={roleTabs.edit.name}
+ key={roleTabs.edit.key}
+ forceRender
+ >
+ <>
+ <RoleNameField />
+ <PermissionsField
+ addDangerToast={addDangerToast}
+ loading={loadingRolePermissions}
+ />
+ <UsersField
+ addDangerToast={addDangerToast}
+ loading={loadingRoleUsers}
+ />
+ <GroupsField
+ addDangerToast={addDangerToast}
+ loading={loadingRoleGroups}
+ />
+ </>
+ </Tabs.TabPane>
+ <Tabs.TabPane tab={roleTabs.users.name} key={roleTabs.users.key}>
+ <TableView
+ columns={userColumns}
+ data={roleUsers}
+ emptyWrapperType={EmptyWrapperType.Small}
/>
- <UsersField
- addDangerToast={addDangerToast}
- loading={loadingRoleUsers}
- />
- <GroupsField
- addDangerToast={addDangerToast}
- loading={loadingRoleGroups}
- />
- </>
- </Tabs.TabPane>
- <Tabs.TabPane tab={roleTabs.users.name} key={roleTabs.users.key}>
- <TableView
- columns={userColumns}
- data={roleUsers}
- emptyWrapperType={EmptyWrapperType.Small}
- />
- </Tabs.TabPane>
- </Tabs>
- );
- }}
+ </Tabs.TabPane>
+ </Tabs>
+ );
+ }) as unknown as ReactNode
Review Comment:
I know that we cast as unknown in tests in general, is there a way to avoid this casting in functional code?
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/CategoricalDeckGLContainer.tsx:
##########
@@ -139,7 +139,8 @@ const CategoricalDeckGLContainer = (props: CategoricalDeckGLContainerProps) => {
const setTooltip = useCallback((tooltip: TooltipProps['tooltip']) => {
const { current } = containerRef;
if (current) {
- current.setTooltip(tooltip);
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ (current as any).setTooltip(tooltip);
Review Comment:
Why are we setting it to any here?
##########
superset-frontend/src/components/Chart/Chart.tsx:
##########
@@ -235,16 +235,13 @@ class Chart extends PureComponent<ChartProps, {}> {
);
}
- handleRenderContainerFailure(
- error: Error,
- info: { componentStack: string } | null,
- ) {
+ handleRenderContainerFailure(error: Error, info: ErrorInfo) {
Review Comment:
What the AI suggest sounds interesting. What do you think?
##########
superset-frontend/src/explore/components/controls/CollectionControl/index.tsx:
##########
@@ -16,17 +16,26 @@
* specific language governing permissions and limitations
* under the License.
*/
-import React, { Component } from 'react';
+import React, { useCallback, useMemo } from 'react';
import { IconTooltip, List } from '@superset-ui/core/components';
import { nanoid } from 'nanoid';
import { t } from '@apache-superset/core/translation';
-import { withTheme, type SupersetTheme } from '@apache-superset/core/theme';
+import { useTheme, type SupersetTheme } from '@apache-superset/core/theme';
import {
- SortableContainer,
- SortableHandle,
- SortableElement,
+ DndContext,
+ closestCenter,
+ useSensor,
+ useSensors,
+ PointerSensor,
+ type DragEndEvent,
+} from '@dnd-kit/core';
+import {
+ SortableContext,
+ verticalListSortingStrategy,
+ useSortable,
arrayMove,
-} from 'react-sortable-hoc';
+} from '@dnd-kit/sortable';
+import { CSS } from '@dnd-kit/utilities';
Review Comment:
Is the scope of this PR to also move this component to dnd kit?
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4252672448
๐ช [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/24458659002) for [156af9a](https://github.com/apache/superset/commit/156af9a)
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4258898878
- The SQL dropdown in the nav menu seems to be misaligned with the others elements
- Format SQL inside SQL lab don't works
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4253254070
* Add or edit display controls - Click on New - click on โAdd display controlโ - the modal disappears and re-appears
* Add 3 display controls, remove one, the modal suddenly changes height. When clicking undo returns to normal height
* Clicking on โEnable cross filtersโ in the native filter left panel - settings closes the popover - should it stay open?
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r3168791579
##########
superset-frontend/src/features/reports/ReportModal/index.tsx:
##########
@@ -366,7 +367,7 @@ function ReportModal({
}}
onError={setCronError}
/>
- <StyledCronError>{cronError}</StyledCronError>
+ <StyledCronError>{cronError as ReactNode}</StyledCronError>
<div
Review Comment:
<!-- Bito Reply -->
This question isnโt related to the pull request. I can only help with questions about the PRโs code or comments.
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4372643953
Its happening!
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4034823231
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2917289224
##########
superset-frontend/src/features/users/UserListModal.tsx:
##########
@@ -133,142 +134,146 @@ function UserListModal({
requiredFields={requiredFields}
initialValues={initialValues}
>
- {(form: FormInstance) => (
- <>
- <FormItem
- name="first_name"
- label={t('First name')}
- rules={[{ required: true, message: t('First name is required') }]}
- >
- <Input
+ {
+ ((form: FormInstance) => (
+ <>
+ <FormItem
name="first_name"
- placeholder={t("Enter the user's first name")}
- />
- </FormItem>
- <FormItem
- name="last_name"
- label={t('Last name')}
- rules={[{ required: true, message: t('Last name is required') }]}
- >
- <Input
+ label={t('First name')}
+ rules={[{ required: true, message: t('First name is required') }]}
+ >
+ <Input
+ name="first_name"
+ placeholder={t("Enter the user's first name")}
+ />
+ </FormItem>
+ <FormItem
name="last_name"
- placeholder={t("Enter the user's last name")}
- />
- </FormItem>
- <FormItem
- name="username"
- label={t('Username')}
- rules={[{ required: true, message: t('Username is required') }]}
- >
- <Input
+ label={t('Last name')}
+ rules={[{ required: true, message: t('Last name is required') }]}
+ >
+ <Input
+ name="last_name"
+ placeholder={t("Enter the user's last name")}
+ />
+ </FormItem>
+ <FormItem
name="username"
- placeholder={t("Enter the user's username")}
- />
- </FormItem>
- <FormItem
- name="active"
- label={t('Is active?')}
- valuePropName="checked"
- >
- <Checkbox
- onChange={checked => {
- form.setFieldsValue({ isActive: checked });
- }}
- />
- </FormItem>
- <FormItem
- name="email"
- label={t('Email')}
- rules={[
- { required: true, message: t('Email is required') },
- {
- type: 'email',
- message: t('Please enter a valid email address'),
- },
- ]}
- >
- <Input name="email" placeholder={t("Enter the user's email")} />
- </FormItem>
- <FormItem
- name="roles"
- label={t('Roles')}
- dependencies={['groups']}
- rules={[atLeastOneRoleOrGroup('groups')]}
- >
- <Select
+ label={t('Username')}
+ rules={[{ required: true, message: t('Username is required') }]}
+ >
+ <Input
+ name="username"
+ placeholder={t("Enter the user's username")}
+ />
+ </FormItem>
+ <FormItem
+ name="active"
+ label={t('Is active?')}
+ valuePropName="checked"
+ >
+ <Checkbox
+ onChange={checked => {
+ form.setFieldsValue({ isActive: checked });
+ }}
+ />
Review Comment:
Since the FormItem already has `valuePropName="checked"`, antd handles the checked state automatically via the form binding
```suggestion
<Checkbox />
```
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4046604415
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4056302696
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r3078981027
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx:
##########
@@ -299,10 +299,12 @@ const AgGridDataTable: FunctionComponent<AgGridTableProps> = memo(
};
const handleColumnHeaderClick = useCallback(
- params => {
+ (params: { column?: { colId?: string; sort?: string } }) => {
const colId = params?.column?.colId;
const sortDir = params?.column?.sort;
- handleColSort(colId, sortDir);
+ if (colId && sortDir) {
+ handleColSort(colId, sortDir);
Review Comment:
**Suggestion:** The new click handler only calls sorting when `sortDir` is truthy, which skips the `null` sort case used to clear sorting from the custom header. This prevents users from unsorting a column and leaves stale sort state in server pagination flows. Allow `null` to pass through (while still ignoring `undefined`) so clear-sort events are handled. [logic error]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ ๏ธ Table V2 server-sorted columns cannot be fully unsorted.
- โ ๏ธ ServerPagination sortBy state remains stale after clear sort.
- โ ๏ธ UI header state diverges from backend sorting behavior.
```
</details>
```suggestion
(params: { column?: { colId?: string; sort?: string | null } }) => {
const colId = params?.column?.colId;
const sortDir = params?.column?.sort;
if (colId && sortDir !== undefined) {
handleColSort(colId, sortDir as string);
```
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Open any "Table V2" (AG Grid Table) visualization in Explore or on a dashboard; this
uses `TableChart` from
`superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx:4-38`,
which renders `<AgGridDataTable>` at `AgGridTableChart.tsx:29-52`.
2. Ensure the chart is configured with server-side pagination so the `serverPagination`
prop is true and `serverPaginationData` is populated (these props are passed to
`<AgGridDataTable>` at `AgGridTableChart.tsx:25-32` and consumed in
`AgGridTable/index.tsx:133-139, 248-255`).
3. In the rendered grid, click a sortable column header multiple times to cycle sorting:
the custom header component `CustomHeader`
(`AgGridTable/components/CustomHeader.tsx:103-120`) calls `applySort`/`clearSort`; when
clearing, `clearSort` sends `onColumnHeaderClicked({ column: { colId: sortKey, sort: null
} })` as seen at `CustomHeader.tsx:25-27`.
4. That callback is wired to `handleColumnHeaderClick` in `AgGridTable/index.tsx:301-308`,
but the current implementation only calls `handleColSort(colId, sortDir)` when `if (colId
&& sortDir)` is truthy; a `sort` value of `null` from `clearSort` fails this check, so
`handleColSort` is never invoked, its `sortDir == null` branch at `index.tsx:262-269`
(which should reset AG Grid state and call `onSortChange([])`) never runs, and
`handleSortByChange` in `AgGridTableChart.tsx:348-17` never receives an empty `sortBy`
arrayโleaving stale server-side sort state even though the header UI shows "Clear Sort"
was applied.
```
</details>
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx
**Line:** 302:306
**Comment:**
*Logic Error: The new click handler only calls sorting when `sortDir` is truthy, which skips the `null` sort case used to clear sorting from the custom header. This prevents users from unsorting a column and leaves stale sort state in server pagination flows. Allow `null` to pass through (while still ignoring `undefined`) so clear-sort events are handled.
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>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38563&comment_hash=df6f935ed45c06bbb741a9d1b01fb0295e70b98bb9f784a25873e0461a9f0e9a&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38563&comment_hash=df6f935ed45c06bbb741a9d1b01fb0295e70b98bb9f784a25873e0461a9f0e9a&reaction=dislike'>๐</a>
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r3086505098
##########
superset-frontend/playwright/components/core/EditableTabs.ts:
##########
@@ -0,0 +1,46 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Tabs } from './Tabs';
+
+/**
+ * EditableTabs component for Ant Design editable-card tabs.
+ *
+ * Mirrors the Superset EditableTabs component (type="editable-card")
+ * which adds add/remove tab functionality to the base Tabs component.
+ *
+ * The add button (.ant-tabs-nav-add) is only rendered when
+ * type="editable-card". If the host component switches to type="card"
+ * (e.g., SQL Lab empty state), use the host page object for that case.
+ */
+export class EditableTabs extends Tabs {
+ /**
+ * Clicks the add-tab button rendered by antd in editable-card mode.
+ */
+ async addTab(): Promise<void> {
+ await this.element.getByRole('button', { name: 'Add tab' }).click();
Review Comment:
**Suggestion:** Clicking the add-tab control by hardcoded accessible name is brittle because this icon-only control's label can vary by Ant Design locale/configuration, causing element lookup failures. Use the structural tab-add selector instead of text-based role matching. [logic error]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ SQL Lab tab-creation E2E test fails in non-English locales.
- โ Subsequent tab-close and tab-state tests fail after addTab().
- โ ๏ธ SQL Lab tab behavior untested under localized Ant Design labels.
- โ ๏ธ Playwright page object becomes brittle to Ant Design changes.
```
</details>
```suggestion
await this.nav.locator('.ant-tabs-nav-add').click();
```
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Run the Playwright SQL Lab E2E tests from
`superset-frontend/playwright/tests/sqllab/sqllab.spec.ts` (e.g., chromium-sqllab
project), which use `SqlLabPage` (defined in
`superset-frontend/playwright/pages/SqlLabPage.ts:36-371`) as the page object.
2. During `test.beforeEach` in `sqllab.spec.ts:46-50`, the test constructs `new
SqlLabPage(page)` and calls `gotoAndReady()`, which in `SqlLabPage.ts:78-82` ensures the
SQL Lab tab strip (`data-test="sql-editor-tabs"`) is visible and at least one editor tab
exists using Ant Design `<EditableTabs>` rendered by `TabbedSqlEditors`
(`superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx:41-75, 224-75`).
3. The test `"creates a new tab via button"` at `sqllab.spec.ts:97-110` calls `await
sqlLabPage.addTab()`, which in `SqlLabPage.ts:245-247` delegates to
`this.editorTabs.addTab()`. `this.editorTabs` is an instance of the Playwright
`EditableTabs` wrapper created in the `SqlLabPage` constructor at `SqlLabPage.ts:54-59`.
4. `EditableTabs.addTab()` in
`superset-frontend/playwright/components/core/EditableTabs.ts:32-38` executes
`this.element.getByRole('button', { name: 'Add tab' }).click();`, where `this.element` is
the `.ant-tabs` root from the base `Tabs` class (`Tabs.ts:29-36, 41-43`). In the actual
React UI, the add-tab control is rendered structurally as the Ant Design add button
`.ant-tabs-nav-add` with a custom `addIcon` (`TabbedSqlEditors/index.tsx:55-71`) and Ant
Design's own i18n determines its accessible name. When the UI is configured for a
non-English Ant Design locale via `useLocale`
(`superset-frontend/src/hooks/useLocale.ts:41-56`) or if Ant Design changes that label,
there is no button whose accessible name is exactly the hardcoded string `"Add tab"` under
the `.ant-tabs` root. Playwright's `getByRole('button', { name: 'Add tab' })` therefore
resolves to zero elements and throws a locator error, causing `"creates a new tab via
button"` and other callers of `SqlLabPage.addTab()` (`sqllab.spec.ts:112-153`) to fail
before any new tab is created.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplaywright%2Fcomponents%2Fcore%2FEditableTabs.ts%0A%2A%2ALine%3A%2A%2A%2037%3A37%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20Clicking%20the%20add-tab%20control%20by%20hardcoded%20accessible%20name%20is%20brittle%20because%20this%20icon-only%20control%27s%20label%20can%20vary%20by%20Ant%20Design%20locale%2Fconfiguration%2C%20causing%20element%20lookup%20failures.%20Use%20the%20structural%20tab-add%20selector%20instead%20of%20text-based%20role%20matching.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0A) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20rev
iew.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplaywright%2Fcomponents%2Fcore%2FEditableTabs.ts%0A%2A%2ALine%3A%2A%2A%2037%3A37%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20Clicking%20the%20add-tab%20control%20by%20hardcoded%20accessible%20name%20is%20brittle%20because%20this%20icon-only%20control%27s%20label%20can%20vary%20by%20Ant%20Design%20locale%2Fconfiguration%2C%20causing%20element%20lookup%20failures.%20Use%20the%20structural%20tab-add%20selector%20instead%20of%20text-based%20role%20matching.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0A)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/playwright/components/core/EditableTabs.ts
**Line:** 37:37
**Comment:**
*Logic Error: Clicking the add-tab control by hardcoded accessible name is brittle because this icon-only control's label can vary by Ant Design locale/configuration, causing element lookup failures. Use the structural tab-add selector instead of text-based role matching.
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>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38563&comment_hash=7bca13c4e7f9314b117892f46cdcdc771fdcc85426465699d282486d290da289&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38563&comment_hash=7bca13c4e7f9314b117892f46cdcdc771fdcc85426465699d282486d290da289&reaction=dislike'>๐</a>
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4258916799
- Inside the query history list the copy button in the SQL column overlap the queries
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4369432201
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Upgrade to React 18 [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #38563:
URL: https://github.com/apache/superset/pull/38563#issuecomment-4372618219
LGTM... let's get this in, then we can address a few lingering issues found as a fast-follow.
1. embedded/index.tsx:192 โ createRoot(appMountPoint).render(...) is called inline with no root reference. The started guard is declared inside the 'port transfer' message handler, so each port-transfer event creates a fresh started=false. A re-handshake โ second createRoot on the same DOM node โ React 18 warns and behavior is undefined. Fix: hoist const root = createRoot(appMountPoint) to module scope and unmount in the existing beforeunload handler.
2. DynamicEditableTitle/index.tsx:84-86 โ useEffect(() => setCurrentTitle(title), [title]) still clobbers in-flight user input if the parent's title prop changes during an edit. The "first keystroke" fix only ensured isEditing flips inside handleChange; this effect ignores isEditing. Fix: gate with if (!isEditing) setCurrentTitle(title).
3. CollectionControl/index.tsx:172-176 โ itemIds falls back to String(i) when keyAccessor returns falsy. Two items can collide on the same id, breaking dnd-kit reordering after a drag (and React reuse via shared key). Fix: generate a stable id (e.g. nanoid) on insert; never derive from index.
4. CollectionControl/index.tsx (drag handle + axis lock) โ attributes/listeners spread onto a <span> wrapping an <Icon> (which has role="img") creates a broken a11y tree and breaks keyboard activation. Also missing axis lock โ original used lockAxis="y", items can now drift horizontally. Fix: spread onto a <button type="button"> via setActivatorNodeRef; add modifiers={[restrictToVerticalAxis]} (requires installing @dnd-kit/modifiers, currently not in package.json). Adding KeyboardSensor would also be a trivial accessibility win.
5. DynamicEditableTitle.regression.test.tsx:73 โ Third "prop changes mid-edit do not clobber" test rerenders. Harness with the same initialTitle="Foo", so the prop the component sees never actually changes. The test does not exercise the bug it claims to guard. Fix: pass a different title prop directly to DynamicEditableTitle via rerender, not via the harness.
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org