rusackas commented on code in PR #39461:
URL: https://github.com/apache/superset/pull/39461#discussion_r3411980866
##########
superset-frontend/packages/superset-ui-core/src/components/ModalTrigger/index.tsx:
##########
@@ -110,7 +113,9 @@ export const ModalTrigger = forwardRef(
className={className}
show={showModal}
onHide={close}
- name={modalTitle}
+ // `name` is used for data-test / telemetry and must be a string;
+ // `title` accepts arbitrary ReactNode for rich rendering.
+ name={typeof modalTitle === 'string' ? modalTitle : undefined}
Review Comment:
This is just a pure FC conversion, no behavior change here. The one
rich-title caller (`TextAreaControl`) was already passing a node via `as any`
on `master`, so `name`/`data-test` was already undefined for it. A dedicated
`modalName` prop is a reasonable idea but out of scope for this lint pass.
##########
superset-frontend/src/components/CopyToClipboard/index.tsx:
##########
@@ -16,126 +16,153 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { Component, cloneElement, ReactElement } from 'react';
+import {
+ cloneElement,
+ isValidElement,
+ type KeyboardEvent,
+ ReactElement,
+ useCallback,
+} from 'react';
import { t } from '@apache-superset/core/translation';
import { css, SupersetTheme } from '@apache-superset/core/theme';
import copyTextToClipboard from 'src/utils/copy';
import { Tooltip } from '@superset-ui/core/components';
import withToasts from '../MessageToasts/withToasts';
import type { CopyToClipboardProps } from './types';
-const defaultProps: Partial<CopyToClipboardProps> = {
- copyNode: <span>{t('Copy')}</span>,
- onCopyEnd: () => {},
- shouldShowText: true,
- wrapped: true,
- tooltipText: t('Copy to clipboard'),
- hideTooltip: false,
-};
+function CopyToClip({
+ copyNode = <span>{t('Copy')}</span>,
+ onCopyEnd = () => {},
+ shouldShowText = true,
+ wrapped = true,
+ tooltipText = t('Copy to clipboard'),
+ hideTooltip = false,
+ disabled,
+ getText,
+ text,
+ addSuccessToast,
+ addDangerToast,
+}: CopyToClipboardProps) {
+ const copyToClipboard = useCallback(
+ (textToCopy: Promise<string>) => {
+ copyTextToClipboard(() => textToCopy)
+ .then(() => {
+ addSuccessToast(t('Copied to clipboard!'));
+ })
+ .catch(() => {
+ addDangerToast(
+ t(
+ 'Sorry, your browser does not support copying. Use Ctrl / Cmd +
C!',
+ ),
+ );
+ })
+ .finally(() => {
+ if (onCopyEnd) onCopyEnd();
+ });
+ },
+ [addSuccessToast, addDangerToast, onCopyEnd],
+ );
-class CopyToClip extends Component<CopyToClipboardProps> {
- static defaultProps = defaultProps;
-
- constructor(props: CopyToClipboardProps) {
- super(props);
- this.copyToClipboard = this.copyToClipboard.bind(this);
- this.onClick = this.onClick.bind(this);
- }
-
- onClick() {
- if (this.props.disabled) {
+ const onClick = useCallback(() => {
+ if (disabled) {
return;
}
- if (this.props.getText) {
- this.props.getText((d: string) => {
- this.copyToClipboard(Promise.resolve(d));
+ if (getText) {
+ getText((d: string) => {
+ copyToClipboard(Promise.resolve(d));
});
} else {
- this.copyToClipboard(Promise.resolve(this.props.text || ''));
+ copyToClipboard(Promise.resolve(text || ''));
}
- }
-
- getDecoratedCopyNode() {
- const copyNode = this.props.copyNode as ReactElement;
- const { disabled } = this.props;
- return cloneElement(copyNode, {
- style: {
- ...copyNode.props.style,
- cursor: disabled ? 'not-allowed' : 'pointer',
- },
- onClick: disabled ? undefined : this.onClick,
- 'aria-disabled': disabled || undefined,
- tabIndex: disabled ? -1 : copyNode.props.tabIndex,
- });
- }
+ }, [disabled, getText, text, copyToClipboard]);
- copyToClipboard(textToCopy: Promise<string>) {
- copyTextToClipboard(() => textToCopy)
- .then(() => {
- this.props.addSuccessToast(t('Copied to clipboard!'));
- })
- .catch(() => {
- this.props.addDangerToast(
- t(
- 'Sorry, your browser does not support copying. Use Ctrl / Cmd +
C!',
- ),
- );
- })
- .finally(() => {
- if (this.props.onCopyEnd) this.props.onCopyEnd();
+ const getDecoratedCopyNode = useCallback(() => {
+ const cursor = disabled ? 'not-allowed' : 'pointer';
+ if (isValidElement(copyNode)) {
+ const node = copyNode as ReactElement;
+ return cloneElement(node, {
+ style: {
+ ...node.props.style,
+ cursor,
+ },
+ onClick: disabled ? undefined : onClick,
Review Comment:
This matches `master`, where `getDecoratedCopyNode` already overwrote
`onClick` with the internal handler via `cloneElement`. The conversion
preserves that contract, so no behavior change to compose around here.
--
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]
