Merge branch 'prevent-empty-custom-lists-names-des-530'

mullvad · Jan 5, 2024 · 70eee39 · 70eee39
2 parents 1b97311 + 2bebcce
commit 70eee39
Show file tree

Hide file tree

Showing 4 changed files with 120 additions and 44 deletions.
diff --git a/gui/locales/messages.pot b/gui/locales/messages.pot
@@ -99,6 +99,9 @@ msgstr ""
 msgid "Default"
 msgstr ""
 
+msgid "Delete list"
+msgstr ""
+
 msgid "Disable"
 msgstr ""
 
@@ -1035,6 +1038,10 @@ msgctxt "select-location-view"
 msgid "Custom lists"
 msgstr ""
 
+msgctxt "select-location-view"
+msgid "Do you want to delete the list <b>%(list)s</b>?"
+msgstr ""
+
 msgctxt "select-location-view"
 msgid "Edit list name"
 msgstr ""

diff --git a/gui/src/renderer/components/select-location/CustomListDialogs.tsx b/gui/src/renderer/components/select-location/CustomListDialogs.tsx
@@ -18,7 +18,7 @@ import { useSelector } from '../../redux/store';
 import * as AppButton from '../AppButton';
 import * as Cell from '../cell';
 import { normalText, tinyText } from '../common-styles';
-import { ModalAlert, ModalMessage } from '../Modal';
+import { ModalAlert, ModalAlertType, ModalMessage } from '../Modal';
 import SimpleInput from '../SimpleInput';
 
 const StyledModalMessage = styled(ModalMessage)({
@@ -201,3 +201,45 @@ export function EditListDialog(props: EditListProps) {
     </ModalAlert>
   );
 }
+
+interface DeleteConfirmDialogProps {
+  list: ICustomList;
+  isOpen: boolean;
+  hide: () => void;
+  confirm: () => void;
+}
+
+// Dialog for changing the name of a custom list.
+export function DeleteConfirmDialog(props: DeleteConfirmDialogProps) {
+  const confirm = useCallback(() => {
+    props.confirm();
+    props.hide();
+  }, []);
+
+  return (
+    <ModalAlert
+      type={ModalAlertType.warning}
+      isOpen={props.isOpen}
+      buttons={[
+        <AppButton.RedButton key="save" onClick={confirm}>
+          {messages.gettext('Delete list')}
+        </AppButton.RedButton>,
+        <AppButton.BlueButton key="cancel" onClick={props.hide}>
+          {messages.gettext('Cancel')}
+        </AppButton.BlueButton>,
+      ]}
+      close={props.hide}>
+      <ModalMessage>
+        {formatHtml(
+          sprintf(
+            messages.pgettext(
+              'select-location-view',
+              'Do you want to delete the list <b>%(list)s</b>?',
+            ),
+            { list: props.list.name },
+          ),
+        )}
+      </ModalMessage>
+    </ModalAlert>
+  );
+}
diff --git a/gui/src/renderer/components/select-location/CustomLists.tsx b/gui/src/renderer/components/select-location/CustomLists.tsx
@@ -10,6 +10,7 @@ import { useBoolean, useStyledRef } from '../../lib/utilityHooks';
 import Accordion from '../Accordion';
 import * as Cell from '../cell';
 import { measurements } from '../common-styles';
+import { BackAction } from '../KeyboardNavigation';
 import SimpleInput from '../SimpleInput';
 import { StyledLocationRowIcon } from './LocationRow';
 import { useRelayListContext } from './RelayListContext';
@@ -52,7 +53,11 @@ const StyledAddListCellButton = styled(StyledCellButton)({
 const StyledSideButtonIcon = styled(Cell.Icon)({
   padding: '3px',
 
-  [`${StyledCellButton}:hover &&, ${StyledAddListCellButton}:hover &&`]: {
+  [`${StyledCellButton}:disabled &&, ${StyledAddListCellButton}:disabled &&`]: {
+    backgroundColor: colors.white40,
+  },
+
+  [`${StyledCellButton}:not(:disabled):hover &&, ${StyledAddListCellButton}:not(:disabled):hover &&`]: {
     backgroundColor: colors.white,
   },
 });
@@ -117,6 +122,7 @@ interface AddListFormProps {
 
 function AddListForm(props: AddListFormProps) {
   const [name, setName] = useState('');
+  const nameValid = name.trim() !== '';
   const [error, setError, unsetError] = useBoolean();
   const containerRef = useStyledRef<HTMLDivElement>();
   const inputRef = useStyledRef<HTMLInputElement>();
@@ -128,16 +134,18 @@ function AddListForm(props: AddListFormProps) {
   }, []);
 
   const createList = useCallback(async () => {
-    try {
-      const result = await props.onCreateList(name);
-      if (result) {
-        setError();
+    if (nameValid) {
+      try {
+        const result = await props.onCreateList(name);
+        if (result) {
+          setError();
+        }
+      } catch (e) {
+        const error = e as Error;
+        log.error('Failed to create list:', error.message);
       }
-    } catch (e) {
-      const error = e as Error;
-      log.error('Failed to create list:', error.message);
     }
-  }, [name, props.onCreateList]);
+  }, [name, props.onCreateList, nameValid]);
 
   const onBlur = useCallback(
     (event: React.FocusEvent<HTMLInputElement>) => {
@@ -162,34 +170,37 @@ function AddListForm(props: AddListFormProps) {
   }, [props.visible]);
 
   return (
-    <Accordion expanded={props.visible} onTransitionEnd={onTransitionEnd}>
-      <StyledCellContainer ref={containerRef}>
-        <StyledInputContainer>
-          <StyledInput
-            ref={inputRef}
-            value={name}
-            onChangeValue={onChange}
-            onSubmitValue={createList}
-            onBlur={onBlur}
-            maxLength={30}
-            $error={error}
-            autoFocus
-          />
-        </StyledInputContainer>
-
-        <StyledAddListCellButton
-          $backgroundColor={colors.blue}
-          $backgroundColorHover={colors.blue80}
-          onClick={createList}>
-          <StyledSideButtonIcon source="icon-check" tintColor={colors.white60} width={18} />
-        </StyledAddListCellButton>
-      </StyledCellContainer>
-      <Cell.CellFooter>
-        <Cell.CellFooterText>
-          {messages.pgettext('select-location-view', 'List names must be unique.')}
-        </Cell.CellFooterText>
-      </Cell.CellFooter>
-    </Accordion>
+    <BackAction disabled={!props.visible} action={props.cancel}>
+      <Accordion expanded={props.visible} onTransitionEnd={onTransitionEnd}>
+        <StyledCellContainer ref={containerRef}>
+          <StyledInputContainer>
+            <StyledInput
+              ref={inputRef}
+              value={name}
+              onChangeValue={onChange}
+              onSubmitValue={createList}
+              onBlur={onBlur}
+              maxLength={30}
+              $error={error}
+              autoFocus
+            />
+          </StyledInputContainer>
+
+          <StyledAddListCellButton
+            $backgroundColor={colors.blue}
+            $backgroundColorHover={colors.blue80}
+            disabled={!nameValid}
+            onClick={createList}>
+            <StyledSideButtonIcon source="icon-check" tintColor={colors.white60} width={18} />
+          </StyledAddListCellButton>
+        </StyledCellContainer>
+        <Cell.CellFooter>
+          <Cell.CellFooterText>
+            {messages.pgettext('select-location-view', 'List names must be unique.')}
+          </Cell.CellFooterText>
+        </Cell.CellFooter>
+      </Accordion>
+    </BackAction>
   );
 }
 

diff --git a/gui/src/renderer/components/select-location/LocationRow.tsx b/gui/src/renderer/components/select-location/LocationRow.tsx
@@ -19,7 +19,7 @@ import ChevronButton from '../ChevronButton';
 import { measurements, normalText } from '../common-styles';
 import ImageView from '../ImageView';
 import RelayStatusIndicator from '../RelayStatusIndicator';
-import { AddToListDialog, EditListDialog } from './CustomListDialogs';
+import { AddToListDialog, DeleteConfirmDialog, EditListDialog } from './CustomListDialogs';
 import {
   CitySpecification,
   CountrySpecification,
@@ -105,6 +105,10 @@ const StyledHoverIconButton = styled.button<IButtonColorProps & { $isLast?: bool
     height: measurements.rowMinHeight,
     appearance: 'none',
 
+    '&&:last-child': {
+      paddingRight: '25px',
+    },
+
     '&&:not(:disabled):hover': {
       backgroundColor: props.$backgroundColor,
     },
@@ -158,19 +162,20 @@ function LocationRow<C extends LocationSpecification>(props: IProps<C>) {
   const { updateCustomList, deleteCustomList } = useAppContext();
   const [addToListDialogVisible, showAddToListDialog, hideAddToListDialog] = useBoolean();
   const [editDialogVisible, showEditDialog, hideEditDialog] = useBoolean();
+  const [deleteDialogVisible, showDeleteDialog, hideDeleteDialog] = useBoolean();
   const background = getButtonColor(props.source.selected, props.level, props.source.disabled);
 
   const customLists = useSelector((state) => state.settings.customLists);
 
   // Expand/collapse should only be available if the expanded property is provided in the source
   const expanded = 'expanded' in props.source ? props.source.expanded : undefined;
   const toggleCollapse = useCallback(() => {
-    if (expanded !== undefined) {
+    if (expanded !== undefined && hasChildren) {
       userInvokedExpand.current = true;
       const callback = expanded ? props.onCollapse : props.onExpand;
       callback(props.source.location);
     }
-  }, [props.onExpand, props.onCollapse, props.source.location, expanded]);
+  }, [props.onExpand, props.onCollapse, props.source.location, expanded, hasChildren]);
 
   const handleClick = useCallback(() => {
     if (!props.source.selected) {
@@ -214,7 +219,7 @@ function LocationRow<C extends LocationSpecification>(props: IProps<C>) {
   }, [customLists, props.source.location]);
 
   // Remove an entire custom list.
-  const onRemoveCustomList = useCallback(async () => {
+  const confirmRemoveCustomList = useCallback(async () => {
     if (props.source.location.customList) {
       try {
         await deleteCustomList(props.source.location.customList);
@@ -269,16 +274,18 @@ function LocationRow<C extends LocationSpecification>(props: IProps<C>) {
             <StyledHoverIconButton onClick={showEditDialog} {...background}>
               <StyledHoverIcon source="icon-edit" />
             </StyledHoverIconButton>
-            <StyledHoverIconButton onClick={onRemoveCustomList} $isLast {...background}>
+            <StyledHoverIconButton onClick={showDeleteDialog} $isLast {...background}>
               <StyledHoverIcon source="icon-close" />
             </StyledHoverIconButton>
           </>
         ) : null}
 
-        {hasChildren ? (
+        {hasChildren ||
+        ('customList' in props.source.location && !('country' in props.source.location)) ? (
           <StyledLocationRowIcon
             as={ChevronButton}
             onClick={toggleCollapse}
+            disabled={!hasChildren}
             up={expanded ?? false}
             aria-label={sprintf(
               expanded === true
@@ -312,6 +319,15 @@ function LocationRow<C extends LocationSpecification>(props: IProps<C>) {
       {'list' in props.source && (
         <EditListDialog list={props.source.list} isOpen={editDialogVisible} hide={hideEditDialog} />
       )}
+
+      {'list' in props.source && (
+        <DeleteConfirmDialog
+          list={props.source.list}
+          isOpen={deleteDialogVisible}
+          hide={hideDeleteDialog}
+          confirm={confirmRemoveCustomList}
+        />
+      )}
     </>
   );
 }