mirror of
https://github.com/apache/superset.git
synced 2026-05-22 00:05:15 +00:00
fix(react18): forwardRef on superset-ui-core wrappers used as tooltip/dropdown triggers
React 18.3 promotes findDOMNode to a deprecation warning. rc-resize-observer
(inside antd Tooltip/Dropdown) falls back to findDOMNode when its child does not
support refs. Without forwardRef on Button, Label, Tooltip, BaseIconComponent,
AsyncIcon, the antd-generated Icons (antdEnhancedIcons + iconOverrides), and
the AsyncIcon jest mock, every Tooltip/Dropdown wrapping these would emit the
warning and fail jest-fail-on-console.
EditableTitle wraps antd Input.TextArea in a Tooltip. TextArea's imperative ref
returns { resizableTextArea, focus, blur } (not a DOM node), so the resize
observer's findDOMNode-fallback fires anyway; wrap in a span to attach a real
DOM ref.
Also drop the empty IconWithoutRef helper in RefreshLabel that was wrapping an
icon in forwardRef without forwarding the ref.
This commit is contained in:
@@ -16,7 +16,7 @@
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { Children, ReactElement, Fragment } from 'react';
|
||||
import { Children, ReactElement, Fragment, forwardRef, Ref } from 'react';
|
||||
import cx from 'classnames';
|
||||
import { Button as AntdButton } from 'antd';
|
||||
import { useTheme } from '@apache-superset/core/theme';
|
||||
@@ -100,7 +100,7 @@ const BUTTON_STYLE_MAP: Record<
|
||||
link: { type: 'link' },
|
||||
};
|
||||
|
||||
export function Button(props: ButtonProps) {
|
||||
function ButtonInner(props: ButtonProps, ref: Ref<HTMLElement>) {
|
||||
const {
|
||||
tooltip,
|
||||
placement,
|
||||
@@ -160,6 +160,7 @@ export function Button(props: ButtonProps) {
|
||||
|
||||
const button = (
|
||||
<AntdButton
|
||||
ref={ref as Ref<HTMLButtonElement & HTMLAnchorElement>}
|
||||
href={disabled ? undefined : href}
|
||||
disabled={disabled}
|
||||
type={antdType}
|
||||
@@ -235,4 +236,6 @@ export function Button(props: ButtonProps) {
|
||||
return button;
|
||||
}
|
||||
|
||||
export const Button = forwardRef<HTMLElement, ButtonProps>(ButtonInner);
|
||||
|
||||
export type { ButtonProps, OnClickHandler };
|
||||
|
||||
@@ -240,7 +240,10 @@ export function EditableTitle({
|
||||
t("You don't have the rights to alter this title.")
|
||||
}
|
||||
>
|
||||
{titleComponent}
|
||||
{/* Wrap in span so the Tooltip can attach a ref to a DOM element.
|
||||
antd's Input.TextArea forwards a non-DOM imperative handle, which
|
||||
triggers a React 18 findDOMNode deprecation warning. */}
|
||||
<span>{titleComponent}</span>
|
||||
</Tooltip>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -165,7 +165,7 @@ import {
|
||||
SlackOutlined,
|
||||
ApiOutlined,
|
||||
} from '@ant-design/icons';
|
||||
import { FC } from 'react';
|
||||
import { ForwardRefExoticComponent, RefAttributes, forwardRef } from 'react';
|
||||
import { IconType } from './types';
|
||||
import { BaseIconComponent } from './BaseIcon';
|
||||
|
||||
@@ -323,19 +323,25 @@ type AntdIconNames = keyof typeof AntdIcons;
|
||||
|
||||
export const antdEnhancedIcons: Record<
|
||||
AntdIconNames,
|
||||
FC<IconType>
|
||||
ForwardRefExoticComponent<IconType & RefAttributes<HTMLSpanElement>>
|
||||
> = Object.keys(AntdIcons)
|
||||
.filter(key => !EXCLUDED_ICONS.some(excluded => key.includes(excluded)))
|
||||
.reduce(
|
||||
(acc, key) => {
|
||||
acc[key as AntdIconNames] = (props: IconType) => (
|
||||
<BaseIconComponent
|
||||
component={AntdIcons[key as AntdIconNames]}
|
||||
fileName={key}
|
||||
{...props}
|
||||
/>
|
||||
acc[key as AntdIconNames] = forwardRef<HTMLSpanElement, IconType>(
|
||||
(props, ref) => (
|
||||
<BaseIconComponent
|
||||
ref={ref}
|
||||
component={AntdIcons[key as AntdIconNames]}
|
||||
fileName={key}
|
||||
{...props}
|
||||
/>
|
||||
),
|
||||
);
|
||||
return acc;
|
||||
},
|
||||
{} as Record<AntdIconNames, FC<IconType>>,
|
||||
{} as Record<
|
||||
AntdIconNames,
|
||||
ForwardRefExoticComponent<IconType & RefAttributes<HTMLSpanElement>>
|
||||
>,
|
||||
);
|
||||
|
||||
@@ -17,12 +17,12 @@
|
||||
* under the License.
|
||||
*/
|
||||
|
||||
import { FC, SVGProps, useEffect, useRef, useState } from 'react';
|
||||
import { FC, SVGProps, forwardRef, useEffect, useRef, useState } from 'react';
|
||||
import TransparentIcon from './svgs/transparent.svg';
|
||||
import { IconType } from './types';
|
||||
import { BaseIconComponent } from './BaseIcon';
|
||||
|
||||
const AsyncIcon = (props: IconType) => {
|
||||
const AsyncIcon = forwardRef<HTMLSpanElement, IconType>((props, ref) => {
|
||||
const [, setLoaded] = useState(false);
|
||||
const ImportedSVG = useRef<FC<SVGProps<SVGSVGElement>>>();
|
||||
const { fileName, customIcons, iconSize, iconColor, viewBox, ...restProps } =
|
||||
@@ -46,6 +46,7 @@ const AsyncIcon = (props: IconType) => {
|
||||
|
||||
return (
|
||||
<BaseIconComponent
|
||||
ref={ref}
|
||||
component={ImportedSVG.current || TransparentIcon}
|
||||
fileName={fileName}
|
||||
customIcons={customIcons}
|
||||
@@ -55,6 +56,6 @@ const AsyncIcon = (props: IconType) => {
|
||||
{...restProps}
|
||||
/>
|
||||
);
|
||||
};
|
||||
});
|
||||
|
||||
export default AsyncIcon;
|
||||
|
||||
@@ -17,6 +17,7 @@
|
||||
* under the License.
|
||||
*/
|
||||
|
||||
import { forwardRef } from 'react';
|
||||
import { css, useTheme, getFontSize } from '@apache-superset/core/theme';
|
||||
import { AntdIconType, BaseIconProps, CustomIconType, IconType } from './types';
|
||||
|
||||
@@ -35,65 +36,65 @@ const genAriaLabel = (fileName: string) => {
|
||||
return name.toLowerCase();
|
||||
};
|
||||
|
||||
export const BaseIconComponent: React.FC<
|
||||
export const BaseIconComponent = forwardRef<
|
||||
HTMLSpanElement,
|
||||
BaseIconProps & Omit<IconType, 'component'>
|
||||
> = ({
|
||||
component: Component,
|
||||
iconColor,
|
||||
iconSize,
|
||||
viewBox,
|
||||
customIcons,
|
||||
fileName,
|
||||
...rest
|
||||
}) => {
|
||||
const theme = useTheme();
|
||||
const whatRole = rest?.onClick ? 'button' : 'img';
|
||||
const ariaLabel = genAriaLabel(fileName || '');
|
||||
const style = {
|
||||
color: iconColor,
|
||||
fontSize: iconSize
|
||||
? `${getFontSize(theme, iconSize)}px`
|
||||
: `${theme.fontSize}px`,
|
||||
cursor: rest?.onClick ? 'pointer' : undefined,
|
||||
};
|
||||
>(
|
||||
(
|
||||
{ component: Component, iconColor, iconSize, viewBox, customIcons, fileName, ...rest },
|
||||
ref,
|
||||
) => {
|
||||
const theme = useTheme();
|
||||
const whatRole = rest?.onClick ? 'button' : 'img';
|
||||
const ariaLabel = genAriaLabel(fileName || '');
|
||||
const style = {
|
||||
color: iconColor,
|
||||
fontSize: iconSize
|
||||
? `${getFontSize(theme, iconSize)}px`
|
||||
: `${theme.fontSize}px`,
|
||||
cursor: rest?.onClick ? 'pointer' : undefined,
|
||||
};
|
||||
|
||||
return customIcons ? (
|
||||
<span
|
||||
role={whatRole}
|
||||
aria-label={ariaLabel}
|
||||
data-test={ariaLabel}
|
||||
css={[
|
||||
css`
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
line-height: 0;
|
||||
vertical-align: middle;
|
||||
`,
|
||||
]}
|
||||
>
|
||||
return customIcons ? (
|
||||
<span
|
||||
ref={ref}
|
||||
role={whatRole}
|
||||
aria-label={ariaLabel}
|
||||
data-test={ariaLabel}
|
||||
css={[
|
||||
css`
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
line-height: 0;
|
||||
vertical-align: middle;
|
||||
`,
|
||||
]}
|
||||
>
|
||||
<Component
|
||||
viewBox={viewBox || '0 0 24 24'}
|
||||
style={style}
|
||||
width={
|
||||
iconSize
|
||||
? `${getFontSize(theme, iconSize) || theme.fontSize}px`
|
||||
: `${theme.fontSize}px`
|
||||
}
|
||||
height={
|
||||
iconSize
|
||||
? `${getFontSize(theme, iconSize) || theme.fontSize}px`
|
||||
: `${theme.fontSize}px`
|
||||
}
|
||||
{...(rest as CustomIconType)}
|
||||
/>
|
||||
</span>
|
||||
) : (
|
||||
<Component
|
||||
viewBox={viewBox || '0 0 24 24'}
|
||||
ref={ref}
|
||||
role={whatRole}
|
||||
style={style}
|
||||
width={
|
||||
iconSize
|
||||
? `${getFontSize(theme, iconSize) || theme.fontSize}px`
|
||||
: `${theme.fontSize}px`
|
||||
}
|
||||
height={
|
||||
iconSize
|
||||
? `${getFontSize(theme, iconSize) || theme.fontSize}px`
|
||||
: `${theme.fontSize}px`
|
||||
}
|
||||
{...(rest as CustomIconType)}
|
||||
aria-label={ariaLabel}
|
||||
data-test={ariaLabel}
|
||||
{...(rest as AntdIconType)}
|
||||
/>
|
||||
</span>
|
||||
) : (
|
||||
<Component
|
||||
role={whatRole}
|
||||
style={style}
|
||||
aria-label={ariaLabel}
|
||||
data-test={ariaLabel}
|
||||
{...(rest as AntdIconType)}
|
||||
/>
|
||||
);
|
||||
};
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
@@ -17,12 +17,16 @@
|
||||
* under the License.
|
||||
*/
|
||||
|
||||
import { FC } from 'react';
|
||||
import { ForwardRefExoticComponent, RefAttributes, forwardRef } from 'react';
|
||||
import { antdEnhancedIcons } from './AntdEnhanced';
|
||||
import AsyncIcon from './AsyncIcon';
|
||||
|
||||
import type { IconType } from './types';
|
||||
|
||||
type IconComponent = ForwardRefExoticComponent<
|
||||
IconType & RefAttributes<HTMLSpanElement>
|
||||
>;
|
||||
|
||||
/**
|
||||
* Filename is going to be inferred from the icon name.
|
||||
* i.e. BigNumberChartTile => assets/images/icons/big_number_chart_tile
|
||||
@@ -58,15 +62,17 @@ const customIcons = [
|
||||
'Undo',
|
||||
] as const;
|
||||
|
||||
type CustomIconType = Record<(typeof customIcons)[number], FC<IconType>>;
|
||||
type CustomIconType = Record<(typeof customIcons)[number], IconComponent>;
|
||||
|
||||
const iconOverrides: CustomIconType = {} as CustomIconType;
|
||||
customIcons.forEach(customIcon => {
|
||||
const fileName = customIcon
|
||||
.replace(/([a-z0-9])([A-Z])/g, '$1_$2')
|
||||
.toLowerCase();
|
||||
iconOverrides[customIcon] = (props: IconType) => (
|
||||
<AsyncIcon customIcons fileName={fileName} {...props} />
|
||||
iconOverrides[customIcon] = forwardRef<HTMLSpanElement, IconType>(
|
||||
(props, ref) => (
|
||||
<AsyncIcon ref={ref} customIcons fileName={fileName} {...props} />
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -74,7 +80,7 @@ export type IconNameType =
|
||||
| keyof typeof antdEnhancedIcons
|
||||
| keyof typeof iconOverrides;
|
||||
|
||||
type IconComponentType = Record<IconNameType, FC<IconType>>;
|
||||
type IconComponentType = Record<IconNameType, IconComponent>;
|
||||
|
||||
export const Icons: IconComponentType = {
|
||||
...antdEnhancedIcons,
|
||||
|
||||
@@ -16,6 +16,7 @@
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { forwardRef } from 'react';
|
||||
import { Tag } from '@superset-ui/core/components/Tag';
|
||||
import { css } from '@emotion/react';
|
||||
import { useTheme, getColorVariants } from '@apache-superset/core/theme';
|
||||
@@ -23,7 +24,7 @@ import { DatasetTypeLabel } from './reusable/DatasetTypeLabel';
|
||||
import { PublishedLabel } from './reusable/PublishedLabel';
|
||||
import type { LabelProps } from './types';
|
||||
|
||||
export function Label(props: LabelProps) {
|
||||
export const Label = forwardRef<HTMLSpanElement, LabelProps>((props, ref) => {
|
||||
const theme = useTheme();
|
||||
// Use Ant Design's motion duration instead of deprecated transitionTiming
|
||||
const {
|
||||
@@ -71,6 +72,7 @@ export function Label(props: LabelProps) {
|
||||
|
||||
return (
|
||||
<Tag
|
||||
ref={ref}
|
||||
onClick={onClick}
|
||||
role={onClick ? 'button' : undefined}
|
||||
style={style}
|
||||
@@ -81,6 +83,6 @@ export function Label(props: LabelProps) {
|
||||
{children}
|
||||
</Tag>
|
||||
);
|
||||
}
|
||||
});
|
||||
export { DatasetTypeLabel, PublishedLabel };
|
||||
export type { LabelType } from './types';
|
||||
|
||||
@@ -16,10 +16,9 @@
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { MouseEventHandler, forwardRef } from 'react';
|
||||
import { MouseEventHandler } from 'react';
|
||||
import { SupersetTheme } from '@apache-superset/core/theme';
|
||||
import { Icons } from '@superset-ui/core/components/Icons';
|
||||
import type { IconType } from '@superset-ui/core/components/Icons/types';
|
||||
import { Tooltip } from '../Tooltip';
|
||||
|
||||
export interface RefreshLabelProps {
|
||||
@@ -32,25 +31,19 @@ const RefreshLabel = ({
|
||||
onClick,
|
||||
tooltipContent,
|
||||
disabled,
|
||||
}: RefreshLabelProps) => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
const IconWithoutRef = forwardRef((props: IconType, ref: any) => (
|
||||
<Icons.SyncOutlined iconSize="l" {...props} />
|
||||
));
|
||||
|
||||
return (
|
||||
<Tooltip title={tooltipContent}>
|
||||
<IconWithoutRef
|
||||
role="button"
|
||||
onClick={disabled ? undefined : onClick}
|
||||
css={(theme: SupersetTheme) => ({
|
||||
cursor: 'pointer',
|
||||
color: theme.colorIcon,
|
||||
'&:hover': { color: theme.colorPrimary },
|
||||
})}
|
||||
/>
|
||||
</Tooltip>
|
||||
);
|
||||
};
|
||||
}: RefreshLabelProps) => (
|
||||
<Tooltip title={tooltipContent}>
|
||||
<Icons.SyncOutlined
|
||||
iconSize="l"
|
||||
role="button"
|
||||
onClick={disabled ? undefined : onClick}
|
||||
css={(theme: SupersetTheme) => ({
|
||||
cursor: 'pointer',
|
||||
color: theme.colorIcon,
|
||||
'&:hover': { color: theme.colorPrimary },
|
||||
})}
|
||||
/>
|
||||
</Tooltip>
|
||||
);
|
||||
|
||||
export default RefreshLabel;
|
||||
|
||||
@@ -16,17 +16,21 @@
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { Tooltip as AntdTooltip } from 'antd';
|
||||
import { forwardRef } from 'react';
|
||||
import { Tooltip as AntdTooltip, type TooltipRef } from 'antd';
|
||||
|
||||
import type { TooltipProps, TooltipPlacement } from './types';
|
||||
|
||||
export const Tooltip = ({ overlayStyle, ...props }: TooltipProps) => (
|
||||
<AntdTooltip
|
||||
styles={{
|
||||
body: { overflow: 'hidden', textOverflow: 'ellipsis' },
|
||||
root: overlayStyle ?? {},
|
||||
}}
|
||||
{...props}
|
||||
/>
|
||||
export const Tooltip = forwardRef<TooltipRef, TooltipProps>(
|
||||
({ overlayStyle, ...props }, ref) => (
|
||||
<AntdTooltip
|
||||
ref={ref}
|
||||
styles={{
|
||||
body: { overflow: 'hidden', textOverflow: 'ellipsis' },
|
||||
root: overlayStyle ?? {},
|
||||
}}
|
||||
{...props}
|
||||
/>
|
||||
),
|
||||
);
|
||||
export type { TooltipProps, TooltipPlacement };
|
||||
|
||||
@@ -16,7 +16,7 @@
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { AriaAttributes } from 'react';
|
||||
import { AriaAttributes, Ref } from 'react';
|
||||
import 'core-js/stable';
|
||||
import 'regenerator-runtime/runtime';
|
||||
import jQuery from 'jquery';
|
||||
@@ -98,31 +98,39 @@ jest.mock('rehype-raw', () => () => jest.fn());
|
||||
// Tests should override this when needed
|
||||
jest.mock('@superset-ui/core/components/Icons/AsyncIcon', () => ({
|
||||
__esModule: true,
|
||||
default: ({
|
||||
fileName,
|
||||
role,
|
||||
'aria-label': ariaLabel,
|
||||
onClick,
|
||||
...rest
|
||||
}: {
|
||||
fileName: string;
|
||||
role?: string;
|
||||
'aria-label'?: AriaAttributes['aria-label'];
|
||||
onClick?: () => void;
|
||||
}) => {
|
||||
// Simple mock that provides the essential attributes for testing
|
||||
const label = ariaLabel || fileName?.replace(/_/g, '-').toLowerCase() || '';
|
||||
return (
|
||||
// eslint-disable-next-line jsx-a11y/no-static-element-interactions
|
||||
<span
|
||||
role={role || (onClick ? 'button' : 'img')}
|
||||
aria-label={label}
|
||||
data-test={label}
|
||||
onClick={onClick}
|
||||
{...rest}
|
||||
/>
|
||||
);
|
||||
},
|
||||
// eslint-disable-next-line global-require
|
||||
default: require('react').forwardRef(
|
||||
(
|
||||
{
|
||||
fileName,
|
||||
role,
|
||||
'aria-label': ariaLabel,
|
||||
onClick,
|
||||
...rest
|
||||
}: {
|
||||
fileName: string;
|
||||
role?: string;
|
||||
'aria-label'?: AriaAttributes['aria-label'];
|
||||
onClick?: () => void;
|
||||
},
|
||||
ref: Ref<HTMLSpanElement>,
|
||||
) => {
|
||||
// Simple mock that provides the essential attributes for testing
|
||||
const label =
|
||||
ariaLabel || fileName?.replace(/_/g, '-').toLowerCase() || '';
|
||||
return (
|
||||
// eslint-disable-next-line jsx-a11y/no-static-element-interactions
|
||||
<span
|
||||
ref={ref}
|
||||
role={role || (onClick ? 'button' : 'img')}
|
||||
aria-label={label}
|
||||
data-test={label}
|
||||
onClick={onClick}
|
||||
{...rest}
|
||||
/>
|
||||
);
|
||||
},
|
||||
),
|
||||
StyledIcon: ({
|
||||
component: Component,
|
||||
role,
|
||||
|
||||
Reference in New Issue
Block a user