From fc36be710f79e65baf33265dd58c74b86e49e0dc Mon Sep 17 00:00:00 2001 From: John Chadwick <86682572+johnwchadwick@users.noreply.github.com> Date: Mon, 13 Jun 2022 15:20:05 +0000 Subject: [PATCH] Make `ButtonProps.onClick` more type-able. (#4853) * Make Button onClick prop more typeable Swaps the order of the arguments so that the optional arg is second. * Review fixes. * removes some implicit any's * NoticeTable to FC might as well, at this point. we're trying to do these as we can as they present themselves * fixes margin for collapse icon (broken before this PR) and removes an unnecessary type parameter Co-authored-by: Dimitri Mitropoulos --- .../src/button/async-button.tsx | 4 +- .../insomnia-components/src/notice-table.tsx | 209 +++++++++--------- .../src/ui/components/base/button.tsx | 8 +- .../src/ui/components/base/copy-button.tsx | 2 +- .../src/ui/components/base/prompt-button.tsx | 22 +- .../auth/components/auth-input-row.tsx | 6 +- .../auth/components/auth-toggle-row.tsx | 5 +- .../ui/components/key-value-editor/row.tsx | 2 +- .../src/ui/components/modals/prompt-modal.tsx | 4 +- .../modals/request-switcher-modal.tsx | 20 +- .../workspace-environments-edit-modal.tsx | 22 +- 11 files changed, 157 insertions(+), 147 deletions(-) diff --git a/packages/insomnia-components/src/button/async-button.tsx b/packages/insomnia-components/src/button/async-button.tsx index 053cddbd86..ca80f48b66 100644 --- a/packages/insomnia-components/src/button/async-button.tsx +++ b/packages/insomnia-components/src/button/async-button.tsx @@ -1,4 +1,4 @@ -import React, { ReactNode, useCallback, useState } from 'react'; +import React, { MouseEvent, ReactNode, useCallback, useState } from 'react'; import type { ButtonProps } from './button'; import { Button } from './button'; @@ -26,7 +26,7 @@ export const AsyncButton = ({ ...props }: AsyncButtonProps) => { const [loading, setLoading] = useState(false); - const asyncHandler = useCallback(async event => { + const asyncHandler = useCallback(async (event: MouseEvent) => { const result = onClick(event); if (isPromise(result)) { diff --git a/packages/insomnia-components/src/notice-table.tsx b/packages/insomnia-components/src/notice-table.tsx index f99afbaf2a..ec11e59a03 100644 --- a/packages/insomnia-components/src/notice-table.tsx +++ b/packages/insomnia-components/src/notice-table.tsx @@ -1,4 +1,4 @@ -import React, { PureComponent } from 'react'; +import React, { PropsWithChildren, useCallback, useState } from 'react'; import styled from 'styled-components'; import { Button } from './button'; @@ -13,16 +13,12 @@ export interface Notice { export interface NoticeTableProps { notices: T[]; - onClick?: (notice: T, event: React.SyntheticEvent) => any; + onClick: (notice: T) => void; onVisibilityToggle?: (expanded: boolean) => any; compact?: boolean; className?: string; } -interface State { - collapsed: boolean; -} - const Wrapper = styled.div` width: 100%; @@ -94,104 +90,109 @@ const Header = styled.header` padding-left: var(--padding-md); `; -export class NoticeTable extends PureComponent, State> { - state: State = { - collapsed: false, - }; +const NoticeRow = ({ + notice, + onClick: propsOnClick, +}: PropsWithChildren<{ + notice: T; + onClick:(notice: T) => void; +}>) => { + const onClick = useCallback(() => { + propsOnClick?.(notice); + }, [notice, propsOnClick]); - collapse() { - const { onVisibilityToggle } = this.props; - this.setState( - state => ({ - collapsed: !state.collapsed, - }), - () => { - if (onVisibilityToggle) { - onVisibilityToggle(!this.state.collapsed); - } - }, - ); - } + return ( + + + + + + {notice.line} + + + + + {notice.message} + + ); +}; - onClick(notice: T, event: React.SyntheticEvent) { - const { onClick } = this.props; - onClick?.(notice, event); - } +export const NoticeTable = ({ + notices, + compact, + onClick, + onVisibilityToggle, +}: PropsWithChildren>) => { + const [collapsed, setCollapsed] = useState(false); - render() { - const { notices, compact } = this.props; - const { collapsed } = this.state; - const caret = collapsed ? ( - - ) : ( - - ); - const errors = notices.filter(n => n.type === 'error'); - const warnings = notices.filter(n => n.type === 'warning'); - return ( - -
-
- {errors.length > 0 && ( - - - - )} - {warnings.length > 0 && ( - - - - )} -
- {/* @ts-expect-error TSCONVERSION */} - -
- {!collapsed && ( - - - - - Type - - Line - - - Message - - - - - {notices.map(notice => ( - - - - - - {notice.line} - - - - - {notice.message} - - ))} - -
- - )} - - ); - } -} + const onCollapse = useCallback(() => { + setCollapsed(!collapsed); + if (onVisibilityToggle) { + onVisibilityToggle(!collapsed); + } + }, [onVisibilityToggle, collapsed]); + + const errors = notices.filter(notice => notice.type === 'error'); + const warnings = notices.filter(notice => notice.type === 'warning'); + return ( + +
+
+ {errors.length > 0 && ( + + + + )} + {warnings.length > 0 && ( + + + + )} +
+ +
+ {!collapsed && ( + + + + + Type + + Line + + + Message + + + + + {notices.map(notice => ( + + ))} + +
+ + )} + + ); +}; diff --git a/packages/insomnia/src/ui/components/base/button.tsx b/packages/insomnia/src/ui/components/base/button.tsx index b322a91fbb..b59d792d37 100644 --- a/packages/insomnia/src/ui/components/base/button.tsx +++ b/packages/insomnia/src/ui/components/base/button.tsx @@ -7,8 +7,8 @@ export interface ButtonProps { children: ReactNode; value?: T; className?: string; - onDisabledClick?: React.MouseEventHandler | ((value: T | undefined, event: React.MouseEvent) => void); - onClick?: React.MouseEventHandler | ((value: T | undefined, event: React.MouseEvent) => void); + onDisabledClick?: (event: React.MouseEvent, value?: T) => void; + onClick?: (event: React.MouseEvent, value?: T) => void; disabled?: boolean; tabIndex?: number; type?: ButtonHTMLAttributes['type']; @@ -24,10 +24,8 @@ export class Button extends PureComponent> { const fn = disabled ? onDisabledClick : onClick; if (this.props.hasOwnProperty('value')) { - // @ts-expect-error -- TSCONVERSION we really need to make the `value` argument come second - fn?.(value, event); + fn?.(event, value); } else { - // @ts-expect-error -- TSCONVERSION we really need to make the `value` argument come second fn?.(event); } } diff --git a/packages/insomnia/src/ui/components/base/copy-button.tsx b/packages/insomnia/src/ui/components/base/copy-button.tsx index c68fa0dcad..82294e4792 100644 --- a/packages/insomnia/src/ui/components/base/copy-button.tsx +++ b/packages/insomnia/src/ui/components/base/copy-button.tsx @@ -24,7 +24,7 @@ export class CopyButton extends PureComponent { _triggerTimeout: NodeJS.Timeout | null = null; - async _handleClick(event) { + async _handleClick(event: React.MouseEvent) { event.preventDefault(); event.stopPropagation(); const content = diff --git a/packages/insomnia/src/ui/components/base/prompt-button.tsx b/packages/insomnia/src/ui/components/base/prompt-button.tsx index dd8da6383a..afb20c840a 100644 --- a/packages/insomnia/src/ui/components/base/prompt-button.tsx +++ b/packages/insomnia/src/ui/components/base/prompt-button.tsx @@ -13,14 +13,14 @@ type States = const STATE_DEFAULT = 'default'; const STATE_ASK = 'ask'; const STATE_DONE = 'done'; -interface Props { - onClick?: React.MouseEventHandler | ((value: any, event: React.MouseEvent) => void); +interface Props { + onClick?: (event: React.MouseEvent, value?: T) => void; addIcon?: boolean; children?: ReactNode; disabled?: boolean; confirmMessage?: string; doneMessage?: string; - value?: any; + value?: T; tabIndex?: number; className?: string; // TODO(TSCONVERSION) this is linked to Button not using title @@ -32,22 +32,21 @@ interface State { } @autoBindMethodsForReact(AUTOBIND_CFG) -export class PromptButton extends PureComponent { +export class PromptButton extends PureComponent> { _doneTimeout: NodeJS.Timeout | null = null; _triggerTimeout: NodeJS.Timeout | null = null; state: State = { state: STATE_DEFAULT, }; - _confirm(...args) { + _confirm(event: React.MouseEvent, value?: T) { if (this._triggerTimeout !== null) { // Clear existing timeouts clearTimeout(this._triggerTimeout); } // Fire the click handler - // @ts-expect-error -- TSCONVERSION - this.props.onClick?.(...args); + this.props.onClick?.(event, value); // Set the state to done (but delay a bit to not alarm user) // using global.setTimeout to force use of the Node timeout rather than DOM timeout this._doneTimeout = global.setTimeout(() => { @@ -64,8 +63,7 @@ export class PromptButton extends PureComponent { }, 2000); } - _ask(...args) { - const event = args[args.length - 1]; + _ask(event: React.MouseEvent) { // Prevent events (ex. won't close dropdown if it's in one) event.preventDefault(); event.stopPropagation(); @@ -82,13 +80,13 @@ export class PromptButton extends PureComponent { }, 2000); } - _handleClick(...args) { + _handleClick(event: React.MouseEvent, value?: T) { const { state } = this.state; if (state === STATE_ASK) { - this._confirm(...args); + this._confirm(event, value); } else if (state === STATE_DEFAULT) { - this._ask(...args); + this._ask(event); } else { // Do nothing } diff --git a/packages/insomnia/src/ui/components/editors/auth/components/auth-input-row.tsx b/packages/insomnia/src/ui/components/editors/auth/components/auth-input-row.tsx index 527da45959..56d3ffa1a8 100644 --- a/packages/insomnia/src/ui/components/editors/auth/components/auth-input-row.tsx +++ b/packages/insomnia/src/ui/components/editors/auth/components/auth-input-row.tsx @@ -24,6 +24,9 @@ export const AuthInputRow: FC = ({ label, getAutocompleteConstants, prope const canBeMasked = !showPasswords && mask; const isMasked = canBeMasked && masked; + // this handler is needed to ignore the parameters sent by button into onClick... + const onClick = useCallback(() => toggleMask(), [toggleMask]); + const onChange = useCallback((value: string) => patchAuth({ [property]: value }), [patchAuth, property]); const id = toKebabCase(label); @@ -42,8 +45,7 @@ export const AuthInputRow: FC = ({ label, getAutocompleteConstants, prope {canBeMasked ? ( diff --git a/packages/insomnia/src/ui/components/modals/workspace-environments-edit-modal.tsx b/packages/insomnia/src/ui/components/modals/workspace-environments-edit-modal.tsx index 08ba5ff3c0..2be801d2bf 100644 --- a/packages/insomnia/src/ui/components/modals/workspace-environments-edit-modal.tsx +++ b/packages/insomnia/src/ui/components/modals/workspace-environments-edit-modal.tsx @@ -53,7 +53,7 @@ const SidebarListItem = SortableElement(({ handleActivateEnvironment, selectedEnvironment, showEnvironment, -}) => { +}: SidebarListItemProps) => { const classes = classnames({ 'env-modal__sidebar-item': true, 'env-modal__sidebar-item--active': selectedEnvironment === environment, @@ -113,7 +113,7 @@ const SidebarList = SortableContainer( handleActivateEnvironment, selectedEnvironment, showEnvironment, - }) => ( + }: SidebarListProps) => (
    {environments.map((environment, index) => ( await this._load(workspace, environment); } - _handleShowEnvironment(environment: Environment) { + _handleShowEnvironment(_event: React.MouseEvent, environment?: Environment) { // Don't allow switching if the current one has errors if (this.environmentEditorRef && !this.environmentEditorRef.isValid()) { return; @@ -234,13 +234,13 @@ export class WorkspaceEnvironmentsEditModal extends PureComponent this._load(workspace, environment); } - async _handleDuplicateEnvironment(environment: Environment) { + async _handleDuplicateEnvironment(_event: React.MouseEvent, environment: Environment) { const { workspace } = this.state; const newEnvironment = await models.environment.duplicate(environment); await this._load(workspace, newEnvironment); } - async _handleDeleteEnvironment(environment: Environment) { + async _handleDeleteEnvironment(_event: React.MouseEvent, environment: Environment) { const { handleChangeEnvironment, activeEnvironmentId } = this.props; const { rootEnvironment, workspace } = this.state; @@ -377,12 +377,12 @@ export class WorkspaceEnvironmentsEditModal extends PureComponent return; } - let patch; + let patch: Partial; try { const data = this.environmentEditorRef.getValue(); patch = { - data: data && data.object, + data: data?.object, dataPropertyOrder: data && data.propertyOrder, }; } catch (err) { @@ -410,15 +410,19 @@ export class WorkspaceEnvironmentsEditModal extends PureComponent this._handleChangeEnvironmentColor(environment, null); } - _handleActivateEnvironment: ButtonProps['onClick'] = (environment: Environment) => { + _handleActivateEnvironment: ButtonProps['onClick'] = (event, environment) => { const { handleChangeEnvironment, activeEnvironmentId } = this.props; + if (!environment) { + return; + } + if (environment._id === activeEnvironmentId) { return; } handleChangeEnvironment(environment._id); - this._handleShowEnvironment(environment); + this._handleShowEnvironment(event, environment); }; render() {