From 860565f3a68be3449daa0b6e1df096b4ece76f33 Mon Sep 17 00:00:00 2001 From: Dimitri Mitropoulos Date: Wed, 19 May 2021 08:24:01 -0400 Subject: [PATCH] fixes react controlled input error (#3391) * fix ts-expect-error * removes useless await see https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/await-thenable.md We're not quite ready to have this rule on everywhere, but in this case I can see that it's correct to follow. * add comment about magic number and moves the assignment to the one place it's used I opted not to make a ticket because we are recently running out of tickets and this seems to be a relatively small thing @reynolek please confirm this is the correct course of action. * initialize ref to reflect reality (and include actual type) * fixes root cause of error, inlines one-liners the error with the type of `_handleInputColorChange` was due to `onChange` being the wrong prop to use (where `onChange` is the one we want). * use onChange instead of onInput to access correct types ultimately, since react's behavior diverged from the DOM behavior to match onInput, this should have no effect on any functionality since react assigns this event to `onChange` anyway. see https://github.com/facebook/react/blob/master/fixtures/attribute-behavior/src/attributes.js#L2089 * clears remaining ts-expect-errors in file * updates fallback call per review feedback --- .../ui/components/base/dropdown/dropdown.tsx | 6 +- .../workspace-environments-edit-modal.tsx | 71 +++++++++---------- .../src/dropdown/dropdown.tsx | 5 +- 3 files changed, 38 insertions(+), 44 deletions(-) diff --git a/packages/insomnia-app/app/ui/components/base/dropdown/dropdown.tsx b/packages/insomnia-app/app/ui/components/base/dropdown/dropdown.tsx index fa1d7c0a83..de36fb3fc7 100644 --- a/packages/insomnia-app/app/ui/components/base/dropdown/dropdown.tsx +++ b/packages/insomnia-app/app/ui/components/base/dropdown/dropdown.tsx @@ -71,8 +71,8 @@ class Dropdown extends PureComponent { } } - _handleChangeFilter(e) { - const newFilter = e.target.value; + _handleChangeFilter(event: React.ChangeEvent) { + const newFilter = event.target.value; // Nothing to do if the filter didn't change if (newFilter === this.state.filter) { @@ -424,7 +424,7 @@ class Dropdown extends PureComponent { diff --git a/packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.tsx b/packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.tsx index 8e08804dfd..8c7919be29 100644 --- a/packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.tsx +++ b/packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.tsx @@ -1,4 +1,4 @@ -import React, { FormEvent, Fragment, PureComponent } from 'react'; +import React, { Fragment, PureComponent } from 'react'; import { autoBindMethodsForReact } from 'class-autobind-decorator'; import { AUTOBIND_CFG, DEBOUNCE_MILLIS } from '../../../common/constants'; import classnames from 'classnames'; @@ -140,7 +140,7 @@ const SidebarList = SortableContainer( @autoBindMethodsForReact(AUTOBIND_CFG) class WorkspaceEnvironmentsEditModal extends PureComponent { environmentEditorRef: EnvironmentEditor | null = null; - environmentColorInputRef: HTMLInputElement; + environmentColorInputRef: HTMLInputElement | null = null; saveTimeout: NodeJS.Timeout | null = null; modal: Modal | null = null; editorKey = 0; @@ -163,10 +163,6 @@ class WorkspaceEnvironmentsEditModal extends PureComponent { this.environmentEditorRef = n; } - _setInputColorRef(ref: HTMLInputElement) { - this.environmentColorInputRef = ref; - } - _setModalRef(n: Modal) { this.modal = n; } @@ -270,10 +266,14 @@ class WorkspaceEnvironmentsEditModal extends PureComponent { } async _updateEnvironment( - environment: Environment, + environment: Environment | null, patch: Partial, refresh = true, ) { + if (environment === null) { + return; + } + const { workspace } = this.state; // NOTE: Fetch the environment first because it might not be up to date. // For example, editing the body updates silently. @@ -301,10 +301,7 @@ class WorkspaceEnvironmentsEditModal extends PureComponent { clearTimeout(this.colorChangeTimeout); } this.colorChangeTimeout = setTimeout(async () => { - // @ts-expect-error -- TSCONVERSION environment can be null - await this._updateEnvironment(environment, { - color, - }); + await this._updateEnvironment(environment, { color }); }, DEBOUNCE_MILLIS); } @@ -331,10 +328,6 @@ class WorkspaceEnvironmentsEditModal extends PureComponent { } } - _handleUnsetColor(environment: Environment) { - this._handleChangeEnvironmentColor(environment, null); - } - componentDidMount() { db.onChange(async changes => { const { selectedEnvironmentId } = this.state; @@ -375,24 +368,15 @@ class WorkspaceEnvironmentsEditModal extends PureComponent { }); } - async _handleClickColorChange(environment: Environment) { - const color = environment.color || '#7d69cb'; - + _handleClickColorChange(environment: Environment) { if (!environment.color) { - this._handleChangeEnvironmentColor(environment, color); + // TODO: fix magic-number. Currently this is the `surprise` background color for the default theme, but we should be grabbing the actual value from the user's actual theme instead. + this._handleChangeEnvironmentColor(environment, '#7d69cb'); } this.environmentColorInputRef?.click(); } - _handleInputColorChange(event: FormEvent) { - this._handleChangeEnvironmentColor( - this._getSelectedEnvironment(), - // @ts-expect-error -- TSCONVERSION what? apparently value doesn't exist on the target - event.target && event.target.value, - ); - } - _saveChanges() { // Only save if it's valid if (!this.environmentEditorRef || !this.environmentEditorRef.isValid()) { @@ -424,6 +408,14 @@ class WorkspaceEnvironmentsEditModal extends PureComponent { } } + handleInputColorChage(event: React.ChangeEvent) { + this._handleChangeEnvironmentColor(this._getSelectedEnvironment(), event.target.value); + } + + unsetColor(environment: Environment) { + this._handleChangeEnvironmentColor(environment, null); + } + _handleActivateEnvironment: ButtonProps['onClick'] = (environment: Environment) => { const { handleChangeEnvironment, activeEnvironmentId } = this.props; @@ -451,6 +443,10 @@ class WorkspaceEnvironmentsEditModal extends PureComponent { const selectedEnvironment = this._getSelectedEnvironment(); + if (this.environmentColorInputRef !== null) { + this.environmentColorInputRef.value = selectedEnvironment?.color || ''; + } + const environmentInfo = { object: selectedEnvironment ? selectedEnvironment.data : {}, propertyOrder: selectedEnvironment && selectedEnvironment.dataPropertyOrder, @@ -513,11 +509,12 @@ class WorkspaceEnvironmentsEditModal extends PureComponent { - selectedEnvironment && - // @ts-expect-error -- TSCONVERSION only set name if defined - this._handleChangeEnvironmentName(selectedEnvironment, name) - } + onSubmit={name => { + if (!selectedEnvironment || !name) { + return; + } + this._handleChangeEnvironmentName(selectedEnvironment, name); + }} value={selectedEnvironment ? selectedEnvironment.name : ''} /> )} @@ -528,9 +525,8 @@ class WorkspaceEnvironmentsEditModal extends PureComponent { { this.environmentColorInputRef = ref; }} + onChange={this.handleInputColorChage} /> @@ -550,8 +546,7 @@ class WorkspaceEnvironmentsEditModal extends PureComponent { {selectedEnvironment.color ? 'Change Color' : 'Assign Color'} @@ -559,7 +554,7 @@ class WorkspaceEnvironmentsEditModal extends PureComponent { Unset Color diff --git a/packages/insomnia-components/src/dropdown/dropdown.tsx b/packages/insomnia-components/src/dropdown/dropdown.tsx index d210bca3ec..8d4d880b49 100644 --- a/packages/insomnia-components/src/dropdown/dropdown.tsx +++ b/packages/insomnia-components/src/dropdown/dropdown.tsx @@ -196,8 +196,7 @@ export class Dropdown extends PureComponent { } } - _handleChangeFilter(event: React.FormEvent) { - // @ts-expect-error -- TSCONVERSION + _handleChangeFilter(event: React.ChangeEvent) { const newFilter = event.target.value; // Nothing to do if the filter didn't change @@ -553,7 +552,7 @@ export class Dropdown extends PureComponent { { this._filter = ref;