Skip to content

fix(CodeEditor): use codeEditorControls and clean up overall #7931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tlabaj merged 12 commits into from
Oct 21, 2022
Merged

fix(CodeEditor): use codeEditorControls and clean up overall #7931

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
257ff8a
fix(CodeEditor): use codeEditorControls and clean up overall
nicolethoen Aug 29, 2022
71ba55a
fix lint errors
nicolethoen Sep 2, 2022
e28609b
use better variable names
nicolethoen Sep 8, 2022
191f1f0
clean up per PR comments
nicolethoen Sep 9, 2022
42494c2
add back useCallback
nicolethoen Sep 12, 2022
6889d36
clean up
nicolethoen Sep 23, 2022
c6e9bd6
clean up
nicolethoen Oct 3, 2022
888aba8
fix lint errors
nicolethoen Oct 5, 2022
95298fa
add console warnings when using deprecated props
nicolethoen Oct 13, 2022
d1bc327
fix lint errors
nicolethoen Oct 14, 2022
44ffa43
update warning messages
nicolethoen Oct 18, 2022
96054c0
fix lint again
nicolethoen Oct 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 44 additions & 58 deletions packages/react-code-editor/src/components/CodeEditor/CodeEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
Popover,
PopoverProps,
Title,
Tooltip,
TooltipPosition
} from '@patternfly/react-core';
import MonacoEditor, { ChangeHandler, EditorDidMount } from 'react-monaco-editor';
Expand All @@ -25,6 +24,7 @@ import CodeIcon from '@patternfly/react-icons/dist/esm/icons/code-icon';
import HelpIcon from '@patternfly/react-icons/dist/esm/icons/help-icon';
import Dropzone from 'react-dropzone';
import { CodeEditorContext } from './CodeEditorUtils';
import { CodeEditorControl } from './CodeEditorControl';

export interface Shortcut {
description: string;
Expand Down Expand Up @@ -440,18 +440,8 @@ export class CodeEditor extends React.Component<CodeEditorProps, CodeEditorState
};

copyCode = () => {
if (this.timer) {
window.clearTimeout(this.timer);
this.setState({ copied: false });
}
this.editor?.focus();
document.execCommand('copy');
this.setState({ copied: true }, () => {
this.timer = window.setTimeout(() => {
this.setState({ copied: false });
this.timer = null;
}, 2500);
});
navigator.clipboard.writeText(this.state.value);
Copy link
Contributor

@mcoker mcoker Sep 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥳

this.setState({ copied: true });
};

download = () => {
Expand Down Expand Up @@ -555,55 +545,51 @@ export class CodeEditor extends React.Component<CodeEditorProps, CodeEditorState
</EmptyState>
));

const tooltipProps = {
position: toolTipPosition,
exitDelay: toolTipDelay,
entryDelay: toolTipDelay,
maxWidth: toolTipMaxWidth,
trigger: 'mouseenter focus',
};

const editorHeader = (
<div className={css(styles.codeEditorHeader)}>
{
<div className={css(styles.codeEditorControls)}>
{isCopyEnabled && (!showEmptyState || !!value) && (
<Tooltip
trigger="mouseenter"
content={<div>{copied ? copyButtonSuccessTooltipText : copyButtonToolTipText}</div>}
exitDelay={copied ? toolTipCopyExitDelay : toolTipDelay}
entryDelay={toolTipDelay}
maxWidth={toolTipMaxWidth}
position={toolTipPosition}
>
<Button onClick={this.copyCode} variant="control" aria-label={copyButtonAriaLabel}>
<CopyIcon />
</Button>
</Tooltip>
)}
{isUploadEnabled && (
<Tooltip
trigger="mouseenter focus click"
content={<div>{uploadButtonToolTipText}</div>}
entryDelay={toolTipDelay}
exitDelay={toolTipDelay}
maxWidth={toolTipMaxWidth}
position={toolTipPosition}
>
<Button onClick={open} variant="control" aria-label={uploadButtonAriaLabel}>
<UploadIcon />
</Button>
</Tooltip>
)}
{isDownloadEnabled && (!showEmptyState || !!value) && (
<Tooltip
trigger="mouseenter focus click"
content={<div>{downloadButtonToolTipText}</div>}
entryDelay={toolTipDelay}
exitDelay={toolTipDelay}
maxWidth={toolTipMaxWidth}
position={toolTipPosition}
>
<Button onClick={this.download} variant="control" aria-label={downloadButtonAriaLabel}>
<DownloadIcon />
</Button>
</Tooltip>
)}
{customControls && (
<CodeEditorContext.Provider value={{ code: value }}>{customControls}</CodeEditorContext.Provider>
)}
<CodeEditorContext.Provider value={{ code: value }}>
{isCopyEnabled && (!showEmptyState || !!value) && (
<CodeEditorControl
icon={<CopyIcon />}
aria-label={copyButtonAriaLabel}
tooltipProps={{
...tooltipProps,
'aria-live': 'polite',
content: <div>{copied ? copyButtonSuccessTooltipText : copyButtonToolTipText}</div>,
exitDelay: copied ? toolTipCopyExitDelay : toolTipDelay,
onTooltipHidden: () => this.setState({copied:false})
}}
onClick={this.copyCode}
/>
)}
{isUploadEnabled && (
<CodeEditorControl
icon={<UploadIcon />}
aria-label={uploadButtonAriaLabel}
tooltipProps={{ content: <div>{uploadButtonToolTipText}</div>, ...tooltipProps }}
onClick={open}
/>
)}
{isDownloadEnabled && (!showEmptyState || !!value) && (
<CodeEditorControl
icon={<DownloadIcon />}
aria-label={downloadButtonAriaLabel}
tooltipProps={{ content: <div>{downloadButtonToolTipText}</div>, ...tooltipProps }}
onClick={this.download}
/>
)}
{customControls && customControls}
</CodeEditorContext.Provider>
</div>
}
{<div className={css(styles.codeEditorHeaderMain)}>{headerMainContent}</div>}
Expand Down
29 changes: 16 additions & 13 deletions packages/react-code-editor/src/components/CodeEditor/CodeEditorControl.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@ import { CodeEditorContext } from './CodeEditorUtils';
*/

export interface CodeEditorControlProps extends Omit<ButtonProps, 'onClick'> {
/** Accessible label for the code editor control. */
/** Accessible label for the code editor control */
'aria-label'?: string;
/** Additional classes added to the code editor control. */
className?: string;
/** Delay in ms before the tooltip appears. */
/** @deprecated Delay in ms before the tooltip appears. */
Copy link
Contributor

@tlabaj tlabaj Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you open an issue to remove these for breaking change release please.

Copy link
Contributor Author

@nicolethoen nicolethoen Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#8214

entryDelay?: number;
/** Delay in ms before the tooltip disappears. */
/** @deprecated Delay in ms before the tooltip disappears. */
exitDelay?: number;
/** Icon rendered inside the code editor control. */
icon: React.ReactNode;
/** Maximum width of the tooltip (default 150px). */
/** @deprecated Maximum width of the tooltip (default 150px). */
maxWidth?: string;
/** Copy button popover position. */
/** @deprecated Copy button popover position. */
position?:
| PopoverPosition
| 'auto'
Expand All @@ -35,25 +35,28 @@ export interface CodeEditorControlProps extends Omit<ButtonProps, 'onClick'> {
| 'left-end'
| 'right-start'
| 'right-end';
/** Text to display in the tooltip. */
toolTipText: React.ReactNode;
/** Event handler for the click of the button. */
/** @deprecated Text to display in the tooltip*/
toolTipText?: React.ReactNode;
/** Event handler for the click of the button */
onClick: (code: string, event?: any) => void;
/** Flag indicating that the button is visible above the code editor. */
isVisible?: boolean;
/** Additional tooltip props forwarded to the Tooltip component */
tooltipProps?: any;
}

export const CodeEditorControl: React.FunctionComponent<CodeEditorControlProps> = ({
icon,
className,
'aria-label': ariaLabel,
toolTipText,
exitDelay = 0,
entryDelay = 300,
maxWidth = '100px',
position = 'top',
exitDelay,
entryDelay,
maxWidth,
position,
onClick = () => {},
isVisible = true,
tooltipProps = {},
...props
}: CodeEditorControlProps) => {
const context = React.useContext(CodeEditorContext);
Expand All @@ -64,12 +67,12 @@ export const CodeEditorControl: React.FunctionComponent<CodeEditorControlProps>

return isVisible ? (
<Tooltip
trigger="mouseenter focus click"
exitDelay={exitDelay}
entryDelay={entryDelay}
maxWidth={maxWidth}
position={position}
content={<div>{toolTipText}</div>}
{...tooltipProps}
>
<Button className={className} onClick={onCustomClick} variant="control" aria-label={ariaLabel} {...props}>
{icon}
Expand Down
2 changes: 1 addition & 1 deletion packages/react-code-editor/src/components/CodeEditor/examples/CodeEditorCustomControl.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const CodeEditorCustomControl: React.FunctionComponent = () => {
<CodeEditorControl
icon={<PlayIcon />}
aria-label="Execute code"
toolTipText="Execute code"
tooltipProps={{ content: 'Execute code' }}
onClick={onExecuteCode}
isVisible={code !== ''}
/>
Expand Down
19 changes: 9 additions & 10 deletions ...ges/react-code-editor/src/components/CodeEditor/examples/CodeEditorShortcutMainHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,22 +36,22 @@ export const CodeEditorShortcutMainHeader: React.FunctionComponent = () => {
];
const shortcutsPopoverProps = {
bodyContent: (
<Grid span={6} hasGutter>
{shortcuts.map(s => (
<>
<Grid span={6} hasGutter key="grid">
{shortcuts.map((shortcut, index) => (
<React.Fragment key={index}>
<GridItem style={{ textAlign: 'right', marginRight: '1em' }}>
{s.keys
.map(k => (
<Chip key={k} isReadOnly>
{k}
{shortcut.keys
.map(key => (
<Chip key={key} isReadOnly>
{key}
</Chip>
))
.reduce((prev, curr) => (
<>{[prev, ' + ', curr]}</>
))}
</GridItem>
<GridItem>{s.description}</GridItem>
</>
<GridItem>{shortcut.description}</GridItem>
</React.Fragment>
))}
</Grid>
),
Expand All @@ -60,7 +60,6 @@ export const CodeEditorShortcutMainHeader: React.FunctionComponent = () => {

return (
<CodeEditor
headerMainContent="Shortcut Example"
shortcutsPopoverProps={shortcutsPopoverProps}
isLanguageLabelVisible
code="Some example content"
Expand Down
36 changes: 7 additions & 29 deletions packages/react-core/src/components/ClipboardCopy/ClipboardCopy.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,7 @@ import { ClipboardCopyToggle } from './ClipboardCopyToggle';
import { ClipboardCopyExpanded } from './ClipboardCopyExpanded';

export const clipboardCopyFunc = (event: React.ClipboardEvent<HTMLDivElement>, text?: React.ReactNode) => {
const clipboard = event.currentTarget.parentElement;
const el = document.createElement('textarea');
el.value = text.toString();
clipboard.appendChild(el);
el.select();
document.execCommand('copy');
clipboard.removeChild(el);
navigator.clipboard.writeText(text.toString());
};

export enum ClipboardCopyVariant {
Expand Down Expand Up @@ -76,7 +70,7 @@ export interface ClipboardCopyProps extends Omit<React.HTMLProps<HTMLDivElement>
exitDelay?: number;
/** Delay in ms before the tooltip appears. */
entryDelay?: number;
/** Delay in ms before the tooltip message switch to hover tip. */
/** @deprecated Delay in ms before the tooltip message switch to hover tip. */
Copy link
Contributor

@tlabaj tlabaj Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this being deprecated? Was its only purpose for the Code editor. Could consumers be relying on it?

Copy link
Contributor Author

@nicolethoen nicolethoen Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's being deprecated because of a subtle change in how the three components with a copy feature are handling toggling their tooltip text between 'copy' and 'copied' (or some similar variant).

Before this change, each of the three copy components were handling it differently. CodeBlock was never changing its 'copied' text back to 'copy'. CodeEditor had it hardcoded so that the text changed after 2500 milliseconds. And ClipBoardCopy was using this switchDelay value.

In the later two cases, these timers caused some weird cases where the 'copied' text would switch back before the tooltip closed and cause the text to quickly update before it closed (sometimes with barely enough time for a user to read it, and that is a bit jarring when it happens). I spent some time trying to tweak timers to figure out how to avoid it, but ultimately, since the user can keep the tooltip open for an indeterminate amount of time by interacting with the tooltip's toggle, it was never going to work ideally with timers.

My solution was to wire all three components to trigger the switch between the 'copied' and 'copy' text only when the close transition has completed. That's now handled internally by the function passed to the onTooltipHidden on line 210 of clipboardCopy.tsx. So the switchDelay prop no longer does anything in ClipboardCopy.

switchDelay?: number;
/** A function that is triggered on clicking the copy button. */
onCopy?: (event: React.ClipboardEvent<HTMLDivElement>, text?: React.ReactNode) => void;
Expand Down Expand Up @@ -111,7 +105,7 @@ export class ClipboardCopy extends React.Component<ClipboardCopyProps, Clipboard
variant: 'inline',
position: PopoverPosition.top,
maxWidth: '150px',
exitDelay: 1600,
exitDelay: 1500,
entryDelay: 300,
switchDelay: 2000,
onCopy: clipboardCopyFunc,
Expand Down Expand Up @@ -210,18 +204,10 @@ export class ClipboardCopy extends React.Component<ClipboardCopyProps, Clipboard
textId={`text-input-${id}`}
aria-label={hoverTip}
onClick={(event: any) => {
if (this.timer) {
window.clearTimeout(this.timer);
this.setState({ copied: false });
}
onCopy(event, this.state.text);
this.setState({ copied: true }, () => {
this.timer = window.setTimeout(() => {
this.setState({ copied: false });
this.timer = null;
}, switchDelay);
});
this.setState({ copied: true });
}}
onTooltipHidden={() => this.setState({copied: false})}
>
{this.state.copied ? clickTip : hoverTip}
</ClipboardCopyButton>
Expand Down Expand Up @@ -263,18 +249,10 @@ export class ClipboardCopy extends React.Component<ClipboardCopyProps, Clipboard
textId={`text-input-${id}`}
aria-label={hoverTip}
onClick={(event: any) => {
if (this.timer) {
window.clearTimeout(this.timer);
this.setState({ copied: false });
}
onCopy(event, this.state.text);
this.setState({ copied: true }, () => {
this.timer = window.setTimeout(() => {
this.setState({ copied: false });
this.timer = null;
}, switchDelay);
});
this.setState({ copied: true });
}}
onTooltipHidden={() => this.setState({copied: false})}
>
{this.state.copied ? clickTip : hoverTip}
</ClipboardCopyButton>
Expand Down
4 changes: 4 additions & 0 deletions packages/react-core/src/components/ClipboardCopy/ClipboardCopyButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ export interface ClipboardCopyButtonProps
'aria-label'?: string;
/** Variant of the copy button */
variant?: 'control' | 'plain';
/** Callback when tooltip's hide transition has finished executing */
onTooltipHidden?: () => void;
}

export const ClipboardCopyButton: React.FunctionComponent<ClipboardCopyButtonProps> = ({
Expand All @@ -56,6 +58,7 @@ export const ClipboardCopyButton: React.FunctionComponent<ClipboardCopyButtonPro
textId,
children,
variant = 'control',
onTooltipHidden = () => {},
...props
}: ClipboardCopyButtonProps) => (
<Tooltip
Expand All @@ -67,6 +70,7 @@ export const ClipboardCopyButton: React.FunctionComponent<ClipboardCopyButtonPro
aria-live="polite"
aria="none"
content={<div>{children}</div>}
onTooltipHidden={onTooltipHidden}
>
<Button
type="button"
Expand Down
11 changes: 3 additions & 8 deletions packages/react-core/src/components/CodeBlock/examples/CodeBlockBasic.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,7 @@ export const BasicCodeBlock: React.FunctionComponent = () => {
const [copied, setCopied] = React.useState(false);

const clipboardCopyFunc = (event, text) => {
const clipboard = event.currentTarget.parentElement;
const el = document.createElement('textarea');
el.value = text.toString();
clipboard.appendChild(el);
el.select();
document.execCommand('copy');
clipboard.removeChild(el);
navigator.clipboard.writeText(text.toString());
};

const onClick = (event, text) => {
Expand All @@ -36,9 +30,10 @@ url: https://raw.githubusercontent.com/Azure-Samples/helm-charts/master/docs`;
textId="code-content"
aria-label="Copy to clipboard"
onClick={e => onClick(e, code)}
exitDelay={600}
exitDelay={copied ? 1500 : 600}
maxWidth="110px"
variant="plain"
onTooltipHidden={() => setCopied(false)}
>
{copied ? 'Successfully copied to clipboard!' : 'Copy to clipboard'}
</ClipboardCopyButton>
Expand Down
Loading