Merge branch 'fix-custom-port'

5 files changed, 102 insertions, 94 deletions

diff --git a/gui/locales/messages.pot b/gui/locales/messages.pot
index 02b2ff4186..514cc8c377 100644
--- a/gui/locales/messages.pot
+++ b/gui/locales/messages.pot
@@ -1510,11 +1510,6 @@ msgctxt "wireguard-settings-view"
 msgid "Set %(wireguard)s MTU value. Valid range: %(min)d - %(max)d."
 msgstr ""
 
-#. The hint displayed below the WireGuard port selector.
-msgctxt "wireguard-settings-view"
-msgid "The automatic setting will randomly choose from a wide range of ports."
-msgstr ""
-
 msgctxt "wireguard-settings-view"
 msgid "The automatic setting will randomly choose from the valid port ranges shown below."
 msgstr ""
diff --git a/gui/src/renderer/components/OpenVpnSettings.tsx b/gui/src/renderer/components/OpenVpnSettings.tsx
index 69a570c356..7c842f6c06 100644
--- a/gui/src/renderer/components/OpenVpnSettings.tsx
+++ b/gui/src/renderer/components/OpenVpnSettings.tsx
@@ -465,7 +465,7 @@ function MssFixSetting() {
         </AriaLabel>
         <AriaInput>
           <Cell.AutoSizingTextInput
-            value={mssfix ? mssfix.toString() : ''}
+            initialValue={mssfix ? mssfix.toString() : ''}
             inputMode={'numeric'}
             maxLength={4}
             placeholder={messages.gettext('Default')}
diff --git a/gui/src/renderer/components/WireguardSettings.tsx b/gui/src/renderer/components/WireguardSettings.tsx
index 19afad9928..dea8c6dbc2 100644
--- a/gui/src/renderer/components/WireguardSettings.tsx
+++ b/gui/src/renderer/components/WireguardSettings.tsx
@@ -162,15 +162,10 @@ function PortSelector() {
     [relaySettings],
   );
 
-  const setCustomPort = useCallback(
-    async (port: string) => {
-      await setWireguardPort(parseInt(port));
-    },
-    [setWireguardPort],
-  );
+  const parseValue = useCallback((port: string) => parseInt(port), []);
 
   const validateValue = useCallback(
-    (value) => allowedPortRanges.some(([start, end]) => value >= start && value <= end),
+    (value: number) => allowedPortRanges.some(([start, end]) => value >= start && value <= end),
     [allowedPortRanges],
   );
 
@@ -187,9 +182,9 @@ function PortSelector() {
           items={wireguardPortItems}
           value={port}
           onSelect={setWireguardPort}
-          onSelectCustom={setCustomPort}
           inputPlaceholder={messages.pgettext('wireguard-settings-view', 'Port')}
           automaticValue={null}
+          parseValue={parseValue}
           modifyValue={removeNonNumericCharacters}
           validateValue={validateValue}
           maxLength={5}
@@ -214,19 +209,6 @@ function PortSelector() {
           }
         />
       </StyledSelectorContainer>
-      <Cell.CellFooter>
-        <AriaDescription>
-          <Cell.CellFooterText>
-            {
-              // TRANSLATORS: The hint displayed below the WireGuard port selector.
-              messages.pgettext(
-                'wireguard-settings-view',
-                'The automatic setting will randomly choose from a wide range of ports.',
-              )
-            }
-          </Cell.CellFooterText>
-        </AriaDescription>
-      </Cell.CellFooter>
     </AriaInputGroup>
   );
 }
@@ -544,7 +526,7 @@ function MtuSetting() {
         </AriaLabel>
         <AriaInput>
           <Cell.AutoSizingTextInput
-            value={mtu ? mtu.toString() : ''}
+            initialValue={mtu ? mtu.toString() : ''}
             inputMode={'numeric'}
             maxLength={4}
             placeholder={messages.gettext('Default')}
diff --git a/gui/src/renderer/components/cell/Input.tsx b/gui/src/renderer/components/cell/Input.tsx
index e68fac424c..bff8364d21 100644
--- a/gui/src/renderer/components/cell/Input.tsx
+++ b/gui/src/renderer/components/cell/Input.tsx
@@ -38,29 +38,50 @@ const StyledInput = styled.input({}, (props: { focused: boolean; valid?: boolean
 
 interface IInputProps extends React.InputHTMLAttributes<HTMLInputElement> {
   value?: string;
+  initialValue?: string;
   validateValue?: (value: string) => boolean;
   modifyValue?: (value: string) => string;
   submitOnBlur?: boolean;
   onSubmitValue?: (value: string) => void;
+  onInvalidValue?: (value: string) => void;
   onChangeValue?: (value: string) => void;
 }
 
+// If value is provided this component behaves like a controlled component.
+// If value isn't provided, then initialValue will be used for the initial value, but updates to
+// initialValue will also cause the internal value to update.
 function InputWithRef(props: IInputProps, forwardedRef: React.Ref<HTMLInputElement>) {
   const {
+    initialValue,
     validateValue,
     modifyValue,
     submitOnBlur,
     onSubmitValue,
+    onInvalidValue,
     onChangeValue,
     ...otherProps
   } = props;
 
-  const [value, setValue] = useState(props.value ?? '');
   const [isFocused, setFocused, setBlurred] = useBoolean(false);
 
+  // internalValue will be used when the component is uncontrolled.
+  const [internalValue, setInternalValue] = useState(props.value ?? props.initialValue ?? '');
+  const value = props.value ?? internalValue;
+
   const inputRef = useRef() as React.RefObject<HTMLInputElement>;
   const combinedRef = useCombinedRefs(inputRef, forwardedRef);
 
+  const onSubmit = useCallback(
+    (value: string) => {
+      if (validateValue?.(value) !== false) {
+        onSubmitValue?.(value);
+      } else {
+        onInvalidValue?.(value);
+      }
+    },
+    [onSubmitValue, onInvalidValue],
+  );
+
   const onFocus = useCallback(
     (event: React.FocusEvent<HTMLInputElement>) => {
       setFocused();
@@ -73,41 +94,52 @@ function InputWithRef(props: IInputProps, forwardedRef: React.Ref<HTMLInputEleme
     (event: React.FocusEvent<HTMLInputElement>) => {
       setBlurred();
       props.onBlur?.(event);
-
-      if (validateValue?.(value) !== false && submitOnBlur) {
-        onSubmitValue?.(value);
+      if (submitOnBlur) {
+        onSubmit(value);
       }
     },
-    [value, props.onBlur, validateValue, onSubmitValue, submitOnBlur],
+    [value, props.onBlur, validateValue, onSubmit, submitOnBlur],
   );
 
   const onChange = useCallback(
     (event: React.ChangeEvent<HTMLInputElement>) => {
       const value = modifyValue?.(event.target.value) ?? event.target.value;
-      setValue(value);
+      if (props.value === undefined) {
+        // Only update the internal value when in uncontrolled mode to not cause unnecessary render
+        // cycles.
+        setInternalValue(value);
+      }
+
       props.onChange?.(event);
       onChangeValue?.(value);
     },
-    [value, modifyValue, props.onSubmit, onSubmitValue],
+    [modifyValue, props.onSubmit],
   );
 
   const onKeyPress = useCallback(
     (event: React.KeyboardEvent<HTMLInputElement>) => {
       if (event.key === 'Enter') {
-        onSubmitValue?.(value);
+        onSubmit(value);
         inputRef.current?.blur();
       }
       props.onKeyPress?.(event);
     },
-    [value, onSubmitValue, inputRef, props.onKeyPress],
+    [value, onSubmit, inputRef, props.onKeyPress],
   );
 
+  // If the the initialValue changes in the uncontrolled mode when the user isn't currently writing,
+  // then we want to update the value.
   useEffect(() => {
-    if (!isFocused && props.value !== undefined && value !== props.value) {
-      setValue(props.value);
-      onChangeValue?.(props.value);
+    if (
+      !isFocused &&
+      props.value === undefined &&
+      props.initialValue !== undefined &&
+      internalValue !== props.initialValue
+    ) {
+      setInternalValue(props.initialValue);
+      onChangeValue?.(props.initialValue);
     }
-  }, [props.value]);
+  }, [props.initialValue]);
 
   const valid = validateValue?.(value);
 
@@ -162,21 +194,12 @@ const StyledAutoSizingTextInputWrapper = styled.div({
 });
 
 function AutoSizingTextInputWithRef(props: IInputProps, forwardedRef: React.Ref<HTMLInputElement>) {
-  const { onChangeValue, onFocus, onBlur, ...otherProps } = props;
+  const { onFocus, onBlur, ...otherProps } = props;
 
-  const [value, setValue] = useState(otherProps.value ?? '');
   const [focused, setFocused, setBlurred] = useBoolean(false);
   const inputRef = useRef() as React.RefObject<HTMLInputElement>;
   const combinedRef = useCombinedRefs(inputRef, forwardedRef);
 
-  const onChangeValueWrapper = useCallback(
-    (value: string) => {
-      setValue(value);
-      onChangeValue?.(value);
-    },
-    [onChangeValue],
-  );
-
   const onBlurWrapper = useCallback(
     (event: React.FocusEvent<HTMLInputElement>) => {
       setBlurred();
@@ -195,6 +218,8 @@ function AutoSizingTextInputWithRef(props: IInputProps, forwardedRef: React.Ref<
 
   const blur = useCallback(() => inputRef.current?.blur(), []);
 
+  const value = inputRef.current?.value;
+
   return (
     <BackAction disabled={!focused} action={blur}>
       <InputFrame focused={focused}>
@@ -202,7 +227,6 @@ function AutoSizingTextInputWithRef(props: IInputProps, forwardedRef: React.Ref<
           <StyledAutoSizingTextInputWrapper>
             <Input
               ref={combinedRef}
-              onChangeValue={onChangeValueWrapper}
               onBlur={onBlurWrapper}
               onFocus={onFocusWrapper}
               {...otherProps}
diff --git a/gui/src/renderer/components/cell/Selector.tsx b/gui/src/renderer/components/cell/Selector.tsx
index 2c522178d6..e225595e22 100644
--- a/gui/src/renderer/components/cell/Selector.tsx
+++ b/gui/src/renderer/components/cell/Selector.tsx
@@ -1,4 +1,4 @@
-import { useCallback, useEffect, useRef, useState } from 'react';
+import { useCallback, useRef, useState } from 'react';
 import styled from 'styled-components';
 
 import { colors } from '../../../config.json';
@@ -182,10 +182,10 @@ const StyledCustomContainer = styled(Cell.Container)((props: StyledCustomContain
 interface SelectorWithCustomItemProps<T, U> extends CommonSelectorProps<T | undefined, U> {
   inputPlaceholder: string;
   onSelect: (value: T | U) => void;
-  onSelectCustom: (value: string) => void;
+  parseValue: (value: string) => T;
+  validateValue?: (value: T) => boolean;
   maxLength?: number;
   selectedCellRef?: React.Ref<HTMLDivElement>;
-  validateValue?: (value: string) => boolean;
   modifyValue?: (value: string) => string;
 }
 
@@ -194,65 +194,70 @@ export function SelectorWithCustomItem<T, U>(props: SelectorWithCustomItemProps<
     value,
     inputPlaceholder,
     onSelect,
-    onSelectCustom,
     maxLength,
     selectedCellRef,
     validateValue,
+    parseValue,
     modifyValue,
     ...otherProps
   } = props;
 
-  // The component needs to keep track of when the custom item should look selected before it has a
-  // value.
-  const [customIsSelectedWithoutValue, setCustomIsSelectedWithoutValue] = useState(false);
-  const inputRef = useRef() as React.RefObject<HTMLInputElement>;
-
-  const itemIsSelected =
+  const isNonCustomItem = (value: T | U | undefined) =>
     props.items.some((item) => item.value === value) || props.automaticValue === value;
-  const customIsSelected = !itemIsSelected || customIsSelectedWithoutValue;
 
-  const handleClick = useCallback(() => {
-    if (!customIsSelected) {
-      setCustomIsSelectedWithoutValue(true);
-      inputRef.current?.focus();
-    }
-  }, [customIsSelected, inputRef.current]);
+  const itemIsSelected = isNonCustomItem(value);
+
+  // Value of custom input. The value is undefined when custom isn't picked.
+  const [customValue, setCustomValue] = useState(itemIsSelected ? undefined : `${value}`);
+  const customIsSelected = customValue !== undefined;
 
-  // Wrap onSelect to be able to catch when a new value is selected during the
-  // customIsSelectedWithoutValue phase.
-  const handleSelectValue = useCallback(
+  const inputRef = useRef() as React.RefObject<HTMLInputElement>;
+
+  const handleClickCustom = useCallback(() => {
+    inputRef.current?.focus();
+    setCustomValue((customValue) => customValue ?? '');
+  }, [customValue, inputRef.current]);
+
+  const handleSelectItem = useCallback(
     (newValue: T | U | undefined) => {
-      if (customIsSelectedWithoutValue && newValue === value) {
-        setCustomIsSelectedWithoutValue(false);
-      } else if (newValue !== undefined) {
+      setCustomValue(undefined);
+
+      onSelect(newValue!);
+    },
+    [value, onSelect],
+  );
+
+  const validateCustomValue = useCallback(
+    (value: string) => validateValue?.(parseValue(value)) ?? true,
+    [parseValue, validateValue],
+  );
+
+  const handleSubmitCustom = useCallback(
+    (newStringValue: string) => {
+      const newValue = parseValue(newStringValue);
+
+      if (isNonCustomItem(newValue)) {
+        handleSelectItem(newValue);
+      } else {
         onSelect(newValue);
       }
     },
-    [customIsSelected, value, onSelect],
+    [value, parseValue, onSelect],
   );
 
-  const handleSubmit = useCallback((value: string) => {
-    if (validateValue?.(value) !== false) {
-      onSelectCustom(value);
-    }
-  }, []);
-
-  // If props.value changes while customIsSelectedWithoutValue then we want to switch to that value
-  // instead.
-  useEffect(() => {
-    if (customIsSelected) {
-      setCustomIsSelectedWithoutValue(false);
-    }
-  }, [value]);
+  const handleInvalidCustom = useCallback(
+    () => setCustomValue(itemIsSelected ? undefined : `${value}`),
+    [itemIsSelected, value],
+  );
 
   return (
     <Selector<T | undefined, U>
       {...otherProps}
-      onSelect={handleSelectValue}
+      onSelect={handleSelectItem}
       value={customIsSelected ? undefined : value}>
       <StyledCustomContainer
         ref={customIsSelected ? props.selectedCellRef : undefined}
-        onClick={handleClick}
+        onClick={handleClickCustom}
         selected={customIsSelected}
         disabled={props.disabled}
         role="option"
@@ -268,13 +273,15 @@ export function SelectorWithCustomItem<T, U>(props: SelectorWithCustomItemProps<
         <AriaInput>
           <Cell.AutoSizingTextInput
             ref={inputRef}
-            value={itemIsSelected || customIsSelectedWithoutValue ? '' : `${props.value}`}
+            value={customValue ?? ''}
             placeholder={inputPlaceholder}
             inputMode={'numeric'}
             maxLength={maxLength ?? 4}
-            onSubmitValue={handleSubmit}
+            onChangeValue={setCustomValue}
+            onSubmitValue={handleSubmitCustom}
+            onInvalidValue={handleInvalidCustom}
             submitOnBlur={true}
-            validateValue={validateValue}
+            validateValue={validateCustomValue}
             modifyValue={modifyValue}
           />
         </AriaInput>