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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
