From d42d2eb0c4ef3b79a12911a33a7464eda1bbf2a8 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Mon, 25 Apr 2022 18:16:19 -0400 Subject: [PATCH] Rewrite withTooltip to be more type correct - Fixes mobx related "too many recursive actions" error - Change all the uses of withTooltips to be functional components Signed-off-by: Sebastian Malton --- src/renderer/components/badge/badge.tsx | 114 ++++++++---------- src/renderer/components/button/button.tsx | 50 ++++---- .../components/dock/create-resource/view.tsx | 4 +- src/renderer/components/dock/editor-panel.tsx | 81 ++++++------- src/renderer/components/icon/icon.tsx | 106 ++++++---------- .../line-progress/line-progress.tsx | 28 +++-- src/renderer/components/menu/menu-actions.tsx | 5 +- .../components/status-brick/status-brick.tsx | 21 +--- .../components/tooltip/withTooltip.tsx | 74 +++++++----- 9 files changed, 222 insertions(+), 261 deletions(-) diff --git a/src/renderer/components/badge/badge.tsx b/src/renderer/components/badge/badge.tsx index 9d184f8651..4e0fa8f16f 100644 --- a/src/renderer/components/badge/badge.tsx +++ b/src/renderer/components/badge/badge.tsx @@ -5,15 +5,13 @@ import styles from "./badge.module.scss"; -import React from "react"; -import { computed, makeObservable, observable } from "mobx"; +import React, { createRef, useState } from "react"; +import { action, observable } from "mobx"; import { observer } from "mobx-react"; import { cssNames } from "../../utils/cssNames"; -import type { TooltipDecoratorProps } from "../tooltip"; import { withTooltip } from "../tooltip"; -import { autoBind } from "../../utils"; -export interface BadgeProps extends React.HTMLAttributes, TooltipDecoratorProps { +export interface BadgeProps extends React.HTMLAttributes { small?: boolean; flat?: boolean; label?: React.ReactNode; @@ -24,66 +22,56 @@ export interface BadgeProps extends React.HTMLAttributes, TooltipDecoratorP // Common handler for all Badge instances document.addEventListener("selectionchange", () => { - Badge.badgeMeta.hasTextSelected ||= (window.getSelection()?.toString().trim().length ?? 0) > 0; + badgeMeta.hasTextSelected ||= (window.getSelection()?.toString().trim().length ?? 0) > 0; }); -@withTooltip -@observer -export class Badge extends React.Component { - static defaultProps: Partial = { - expandable: true, - }; +const badgeMeta = observable({ + hasTextSelected: false, +}); - static badgeMeta = observable({ - hasTextSelected: false, +export const Badge = withTooltip(observer(({ + small, + flat, + label, + expandable = false, + disabled, + scrollable, + className, + children, + ...elemProps +}: BadgeProps) => { + const elem = createRef(); + const [isExpanded, setIsExpanded] = useState(false); + + const isExpandable = expandable && elem.current + ? elem.current.clientWidth < elem.current.scrollWidth + : false; + + const onMouseUp = action(() => { + if (!isExpandable || badgeMeta.hasTextSelected) { + badgeMeta.hasTextSelected = false; + } else { + setIsExpanded(!isExpanded); + } }); - @observable.ref elem: HTMLDivElement | null = null; - @observable isExpanded = false; - - constructor(props: BadgeProps) { - super(props); - makeObservable(this); - autoBind(this); - } - - @computed get isExpandable() { - return this.props.expandable && this.elem - ? this.elem.clientWidth < this.elem.scrollWidth - : false; - } - - onMouseUp() { - if (!this.isExpandable || Badge.badgeMeta.hasTextSelected) { - Badge.badgeMeta.hasTextSelected = false; - } else { - this.isExpanded = !this.isExpanded; - } - } - - render() { - const { className, label, disabled, scrollable, small, children, flat, expandable, ...elemProps } = this.props; - const clickable = Boolean(this.props.onClick) || this.isExpandable; - const classNames = cssNames(styles.badge, className, { - [styles.small]: small, - [styles.flat]: flat, - [styles.clickable]: clickable, - [styles.interactive]: this.isExpandable, - [styles.isExpanded]: this.isExpanded, - [styles.disabled]: disabled, - [styles.scrollable]: scrollable, - }); - - return ( -
this.elem = elem} - > - {label} - {children} -
- ); - } -} + return ( +
+ {label} + {children} +
+ ); +})); diff --git a/src/renderer/components/button/button.tsx b/src/renderer/components/button/button.tsx index c5dcc860f1..756f50a584 100644 --- a/src/renderer/components/button/button.tsx +++ b/src/renderer/components/button/button.tsx @@ -7,10 +7,9 @@ import "./button.scss"; import type { ButtonHTMLAttributes } from "react"; import React from "react"; import { cssNames } from "../../utils"; -import type { TooltipDecoratorProps } from "../tooltip"; import { withTooltip } from "../tooltip"; -export interface ButtonProps extends ButtonHTMLAttributes, TooltipDecoratorProps { +export interface ButtonProps extends ButtonHTMLAttributes { label?: React.ReactNode; waiting?: boolean; primary?: boolean; @@ -26,36 +25,33 @@ export interface ButtonProps extends ButtonHTMLAttributes, TooltipDecorator target?: "_blank"; // in case of using @href } -@withTooltip -export class Button extends React.PureComponent { - render() { - const { - waiting, label, primary, accent, plain, hidden, active, big, - round, outlined, tooltip, light, children, tooltipOverrideDisabled, ...btnProps - } = this.props; +export const Button = withTooltip((props: ButtonProps) => { + const { + waiting, label, primary, accent, plain, hidden, active, big, + round, outlined, light, children, ...btnProps + } = props; - if (hidden) return null; + if (hidden) return null; - btnProps.className = cssNames("Button", btnProps.className, { - waiting, primary, accent, plain, active, big, round, outlined, light, - }); + btnProps.className = cssNames("Button", btnProps.className, { + waiting, primary, accent, plain, active, big, round, outlined, light, + }); - // render as link - if (this.props.href) { - return ( - - {label} - {children} - - ); - } - - // render as button + // render as link + if (props.href) { return ( - + ); } -} + + // render as button + return ( + + ); +}); diff --git a/src/renderer/components/dock/create-resource/view.tsx b/src/renderer/components/dock/create-resource/view.tsx index 2dac4e03a9..711397c752 100644 --- a/src/renderer/components/dock/create-resource/view.tsx +++ b/src/renderer/components/dock/create-resource/view.tsx @@ -18,7 +18,7 @@ import * as resourceApplierApi from "../../../../common/k8s-api/endpoints/resour import { Notifications } from "../../notifications"; import logger from "../../../../common/logger"; import type { ApiManager } from "../../../../common/k8s-api/api-manager"; -import { isString, prevDefault } from "../../../utils"; +import { isObject, prevDefault } from "../../../utils"; import { withInjectables } from "@ogre-tools/injectable-react"; import createResourceTabStoreInjectable from "./store.injectable"; import createResourceTemplatesInjectable from "./create-resource-templates.injectable"; @@ -77,7 +77,7 @@ class NonInjectedCreateResource extends React.Component = { - autoFocus: true, -}; +const NonInjectedEditorPanel = observer(({ + dockStore, + onChange, + tabId, + value, + autoFocus = true, + className, + onError, +}: Dependencies & EditorPanelProps) => { + const editor = createRef(); -@observer -class NonInjectedEditorPanel extends React.Component { - static defaultProps = defaultProps as object; - - private readonly editor = React.createRef(); - - constructor(props: EditorPanelProps & Dependencies) { - super(props); - makeObservable(this); - } - - componentDidMount() { - disposeOnUnmount(this, [ - // keep focus on editor's area when just opened - reaction(() => this.props.dockStore.isOpen, isOpen => isOpen && this.editor.current?.focus(), { + useEffect(() => disposer( + reaction( + () => dockStore.isOpen, + isOpen => isOpen && editor.current?.focus(), + { fireImmediately: true, - }), + }, + ), + dockStore.onResize(throttle(() => editor.current?.focus(), 250)), + )); - // focus to editor on dock's resize or turning into fullscreen mode - this.props.dockStore.onResize(throttle(() => this.editor.current?.focus(), 250)), - ]); + if (!tabId) { + return null; } - render() { - const { className, autoFocus, tabId, value, onChange, onError } = this.props; - - if (!tabId) return null; - - return ( - - ); - } -} + return ( + + ); +}); export const EditorPanel = withInjectables(NonInjectedEditorPanel, { getProps: (di, props) => ({ diff --git a/src/renderer/components/icon/icon.tsx b/src/renderer/components/icon/icon.tsx index 381de15e49..5344ac528f 100644 --- a/src/renderer/components/icon/icon.tsx +++ b/src/renderer/components/icon/icon.tsx @@ -9,13 +9,12 @@ import type { ReactNode } from "react"; import React, { createRef } from "react"; import { NavLink } from "react-router-dom"; import type { LocationDescriptor } from "history"; -import { autoBind, cssNames } from "../../utils"; -import type { TooltipDecoratorProps } from "../tooltip"; +import { cssNames } from "../../utils"; import { withTooltip } from "../tooltip"; import isNumber from "lodash/isNumber"; import { decode } from "../../../common/utils/base64"; -export interface IconProps extends React.HTMLAttributes, TooltipDecoratorProps { +export interface IconProps extends React.HTMLAttributes { material?: string; // material-icon, see available names at https://material.io/icons/ svg?: string; // svg-filename without extension in current folder link?: LocationDescriptor; // render icon as NavLink from react-router-dom @@ -31,67 +30,38 @@ export interface IconProps extends React.HTMLAttributes, TooltipDecoratorPr disabled?: boolean; } -@withTooltip -export class Icon extends React.PureComponent { - private readonly ref = createRef(); +export const Icon = Object.assign( + withTooltip((props: IconProps) => { + const ref = createRef(); - static defaultProps: IconProps = { - focusable: true, - }; - - static isSvg(content: string) { - return String(content).includes("svg+xml"); // data-url for raw svg-icon - } - - constructor(props: IconProps) { - super(props); - autoBind(this); - } - - get isInteractive() { - const { interactive, onClick, href, link } = this.props; - - return interactive ?? !!(onClick || href || link); - } - - onClick(evt: React.MouseEvent) { - if (this.props.disabled) { - return; - } - - if (this.props.onClick) { - this.props.onClick(evt); - } - } - - onKeyDown(evt: React.KeyboardEvent) { - switch (evt.nativeEvent.code) { - case "Space": - - // fallthrough - case "Enter": { - this.ref.current?.click(); - evt.preventDefault(); - break; - } - } - - if (this.props.onKeyDown) { - this.props.onKeyDown(evt); - } - } - - render() { - const { isInteractive } = this; const { - // skip passing props to icon's html element + // skip passing props to icon's html element className, href, link, material, svg, size, smallest, small, big, disabled, sticker, active, focusable, children, - interactive: _interactive, - onClick: _onClick, - onKeyDown: _onKeyDown, + interactive, onClick, onKeyDown, ...elemProps - } = this.props; + } = props; + const isInteractive = interactive ?? !!(onClick || href || link); + + const boundOnClick = (event: React.MouseEvent) => { + if (!disabled) { + onClick?.(event); + } + }; + const boundOnKeyDown = (event: React.KeyboardEvent) => { + switch (event.nativeEvent.code) { + case "Space": + + // fallthrough + case "Enter": { + ref.current?.click(); + event.preventDefault(); + break; + } + } + + onKeyDown?.(event); + }; let iconContent: ReactNode; const iconProps: Partial = { @@ -99,8 +69,8 @@ export class Icon extends React.PureComponent { { svg, material, interactive: isInteractive, disabled, sticker, active, focusable }, !size ? { smallest, small, big } : {}, ), - onClick: isInteractive ? this.onClick : undefined, - onKeyDown: isInteractive ? this.onKeyDown : undefined, + onClick: isInteractive ? boundOnClick : undefined, + onKeyDown: isInteractive ? boundOnKeyDown : undefined, tabIndex: isInteractive && focusable && !disabled ? 0 : undefined, style: size ? { "--size": size + (isNumber(size) ? "px" : "") } as React.CSSProperties : undefined, ...elemProps, @@ -137,7 +107,7 @@ export class Icon extends React.PureComponent { {children} @@ -149,11 +119,15 @@ export class Icon extends React.PureComponent { ); } - return ; - } -} + return ; + }), + { + // data-url for raw svg-icon + isSvg: (content: string) => String(content).includes("svg+xml"), + }, +); diff --git a/src/renderer/components/line-progress/line-progress.tsx b/src/renderer/components/line-progress/line-progress.tsx index 765b11a69c..24ef169176 100644 --- a/src/renderer/components/line-progress/line-progress.tsx +++ b/src/renderer/components/line-progress/line-progress.tsx @@ -6,10 +6,9 @@ import "./line-progress.scss"; import React from "react"; import { cssNames } from "../../utils"; -import type { TooltipDecoratorProps } from "../tooltip"; import { withTooltip } from "../tooltip"; -export interface LineProgressProps extends React.HTMLProps, TooltipDecoratorProps { +export interface LineProgressProps extends React.HTMLProps { value: number; min?: number; max?: number; @@ -17,6 +16,10 @@ export interface LineProgressProps extends React.HTMLProps, TooltipDecorato precise?: number; } +function valuePercent({ value, min, max, precise }: Required>) { + return Math.min(100, value / (max - min) * 100).toFixed(precise); +} + export const LineProgress = withTooltip(({ className, min = 0, @@ -25,13 +28,14 @@ export const LineProgress = withTooltip(({ precise = 2, children, ...props -}: LineProgressProps) => { - const valuePercents = Math.min(100, value / (max - min) * 100).toFixed(precise); - - return ( -
-
- {children} -
- ); -}); +}: LineProgressProps) => ( +
+
+ {children} +
+)); diff --git a/src/renderer/components/menu/menu-actions.tsx b/src/renderer/components/menu/menu-actions.tsx index 03ba3ea161..2dcbb5ecd5 100644 --- a/src/renderer/components/menu/menu-actions.tsx +++ b/src/renderer/components/menu/menu-actions.tsx @@ -16,12 +16,13 @@ import type { MenuProps } from "./menu"; import { Menu, MenuItem } from "./menu"; import uniqueId from "lodash/uniqueId"; import isString from "lodash/isString"; +import type { TooltipDecoratorProps } from "../tooltip"; export interface MenuActionsProps extends Partial { className?: string; toolbar?: boolean; // display menu as toolbar with icons autoCloseOnSelect?: boolean; - triggerIcon?: string | IconProps | React.ReactNode; + triggerIcon?: string | (IconProps & TooltipDecoratorProps) | React.ReactNode; removeConfirmationMessage?: React.ReactNode | (() => React.ReactNode); updateAction?: () => void | Promise; removeAction?: () => void | Promise; @@ -75,7 +76,7 @@ export class MenuActions extends React.Component { return React.cloneElement(triggerIcon, { id: this.id, className }); } - const iconProps: Partial = { + const iconProps: IconProps & TooltipDecoratorProps = { id: this.id, interactive: true, material: isString(triggerIcon) ? triggerIcon : undefined, diff --git a/src/renderer/components/status-brick/status-brick.tsx b/src/renderer/components/status-brick/status-brick.tsx index 130b7b68d3..42fcdc98c3 100644 --- a/src/renderer/components/status-brick/status-brick.tsx +++ b/src/renderer/components/status-brick/status-brick.tsx @@ -7,22 +7,13 @@ import "./status-brick.scss"; import React from "react"; import { cssNames } from "../../utils"; -import type { TooltipDecoratorProps } from "../tooltip"; import { withTooltip } from "../tooltip"; -export interface StatusBrickProps extends React.HTMLAttributes, TooltipDecoratorProps { +export interface StatusBrickProps extends React.HTMLAttributes { } -@withTooltip -export class StatusBrick extends React.Component { - render() { - const { className, ...elemProps } = this.props; - - return ( -
- ); - } -} +export const StatusBrick = withTooltip(({ className, ...elemProps }: StatusBrickProps) => ( +
+)); diff --git a/src/renderer/components/tooltip/withTooltip.tsx b/src/renderer/components/tooltip/withTooltip.tsx index 21c2752f7f..013b09cda7 100644 --- a/src/renderer/components/tooltip/withTooltip.tsx +++ b/src/renderer/components/tooltip/withTooltip.tsx @@ -5,13 +5,14 @@ // Tooltip decorator for simple composition with other components -import type { HTMLAttributes, ReactNode } from "react"; -import React from "react"; +import type { ReactNode } from "react"; +import React, { useState } from "react"; import hoistNonReactStatics from "hoist-non-react-statics"; import type { TooltipProps } from "./tooltip"; import { Tooltip } from "./tooltip"; import { isReactNode } from "../../utils/isReactNode"; import uniqueId from "lodash/uniqueId"; +import type { SingleOrMany } from "../../utils"; export interface TooltipDecoratorProps { tooltip?: ReactNode | Omit; @@ -20,41 +21,54 @@ export interface TooltipDecoratorProps { * useful for displaying tooltips even when the target is "disabled" */ tooltipOverrideDisabled?: boolean; + id?: string | undefined; + children?: SingleOrMany; } -export function withTooltip>(Target: T): T { - const DecoratedComponent = class extends React.Component & TooltipDecoratorProps> { - static displayName = `withTooltip(${Target.displayName || Target.name})`; - +export function withTooltip>(Target: React.FunctionComponent): React.FunctionComponent { + const DecoratedComponent = (props: TargetProps & TooltipDecoratorProps) => { // TODO: Remove side-effect to allow deterministic unit testing - protected tooltipId = uniqueId("tooltip_target_"); + const [defaultTooltipId] = useState(uniqueId("tooltip_target_")); - render() { - const { tooltip, tooltipOverrideDisabled, ...targetProps } = this.props; + let { + id: targetId, + children: targetChildren, + } = props; + const { + tooltip, + tooltipOverrideDisabled, + id: _unusedId, + children: _unusedTargetChildren, + ...targetProps + } = props; - if (tooltip) { - const tooltipId = targetProps.id || this.tooltipId; - const tooltipProps: TooltipProps = { - targetId: tooltipId, - tooltipOnParentHover: tooltipOverrideDisabled, - formatters: { narrow: true }, - ...(isReactNode(tooltip) ? { children: tooltip } : tooltip), - }; + if (tooltip) { + const tooltipProps: TooltipProps = { + targetId: targetId || defaultTooltipId, + tooltipOnParentHover: tooltipOverrideDisabled, + formatters: { narrow: true }, + ...(isReactNode(tooltip) ? { children: tooltip } : tooltip), + }; - targetProps.id = tooltipId; - targetProps.children = ( - <> -
- {targetProps.children} -
- - - ); - } - - return ; + targetId = tooltipProps.targetId; + targetChildren = ( + <> +
+ {targetChildren} +
+ + + ); } + + return ( + + {targetChildren} + + ); }; - return hoistNonReactStatics(DecoratedComponent, Target) as any; + DecoratedComponent.displayName = `withTooltip(${Target.displayName || Target.name})`; + + return hoistNonReactStatics(DecoratedComponent, Target); }