Always show the value list + hide "add values" on non-free text entries (#9817)

Removes the condition to hide the value list if we use legal values. 

In doing so, I also realized that focus handling when you delete the
last item in the constraint values list doesn't work if the add values
button isn't there (which it shouldn't be for legal values and more). So
I've hidden the add values button when it doesn't do anythnig helpful
(or for cases where we don't have designs yet). In cases where you don't
have the add values button and you delete the last constraint value,
we'll move the focus to the "delete constraint" button (that was easier
than making sure we pass refs all the way down into the operator select,
but we can change that later).

To facilitate this (refs coming from the parent component), I refactored
the value list component to accept the add values widget as a child (and
extracted it to its own file).

frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/AddValuesWidget.tsx

@@ -0,0 +1,166 @@
+import Add from '@mui/icons-material/Add';
+import { Button, Popover, styled, TextField } from '@mui/material';
+import { ScreenReaderOnly } from 'component/common/ScreenReaderOnly/ScreenReaderOnly';
+import {
+    forwardRef,
+    useId,
+    useImperativeHandle,
+    useRef,
+    useState,
+} from 'react';
+import { parseParameterStrings } from 'utils/parseParameter';
+
+const AddValuesButton = styled('button')(({ theme }) => ({
+    color: theme.palette.primary.main,
+    svg: {
+        fill: theme.palette.primary.main,
+        height: theme.fontSizes.smallerBody,
+        width: theme.fontSizes.smallerBody,
+    },
+    border: 'none',
+    borderRadius: theme.shape.borderRadiusExtraLarge,
+    display: 'flex',
+    flexFlow: 'row nowrap',
+    whiteSpace: 'nowrap',
+    gap: theme.spacing(0.25),
+    alignItems: 'center',
+    padding: theme.spacing(0.5, 1.5, 0.5, 1.5),
+    height: 'auto',
+    transition: 'all 0.3s ease',
+    outline: `1px solid #0000`,
+    background: theme.palette.background.elevation1,
+    ':hover, :focus-visible': {
+        background: theme.palette.background.elevation1,
+        outlineColor: theme.palette.secondary.dark,
+    },
+}));
+
+const StyledPopover = styled(Popover)(({ theme }) => ({
+    '& .MuiPaper-root': {
+        borderRadius: theme.shape.borderRadiusLarge,
+        border: `1px solid ${theme.palette.divider}`,
+        padding: theme.spacing(2),
+        width: '250px',
+    },
+}));
+
+const StyledTextField = styled(TextField)(({ theme }) => ({
+    flexGrow: 1,
+}));
+
+const InputRow = styled('div')(({ theme }) => ({
+    display: 'flex',
+    gap: theme.spacing(1),
+    alignItems: 'start',
+    width: '100%',
+}));
+
+const ErrorMessage = styled('div')(({ theme }) => ({
+    color: theme.palette.error.main,
+    fontSize: theme.typography.caption.fontSize,
+    marginBottom: theme.spacing(1),
+}));
+
+interface AddValuesProps {
+    onAddValues: (newValues: string[]) => void;
+}
+
+export const AddValuesWidget = forwardRef<HTMLButtonElement, AddValuesProps>(
+    ({ onAddValues }, ref) => {
+        const [open, setOpen] = useState(false);
+        const [inputValues, setInputValues] = useState('');
+        const [error, setError] = useState('');
+        const positioningRef = useRef<HTMLButtonElement>(null);
+        useImperativeHandle(
+            ref,
+            () => positioningRef.current as HTMLButtonElement,
+        );
+        const inputRef = useRef<HTMLInputElement>(null);
+        const inputId = useId();
+
+        const handleAdd = () => {
+            const newValues = parseParameterStrings(inputValues);
+
+            if (newValues.length === 0) {
+                setError('Values cannot be empty');
+                return;
+            }
+
+            if (newValues.some((v) => v.length > 100)) {
+                setError('Values cannot be longer than 100 characters');
+                return;
+            }
+
+            onAddValues(newValues);
+            setInputValues('');
+            setError('');
+            inputRef?.current?.focus();
+        };
+
+        return (
+            <>
+                <AddValuesButton
+                    ref={positioningRef}
+                    onClick={() => setOpen(true)}
+                    type='button'
+                >
+                    <Add />
+                    <span>Add values</span>
+                </AddValuesButton>
+                <StyledPopover
+                    open={open}
+                    disableScrollLock
+                    anchorEl={positioningRef.current}
+                    onClose={() => setOpen(false)}
+                    anchorOrigin={{
+                        vertical: 'bottom',
+                        horizontal: 'left',
+                    }}
+                    transformOrigin={{
+                        vertical: 'top',
+                        horizontal: 'left',
+                    }}
+                >
+                    <form
+                        onSubmit={(e) => {
+                            e.stopPropagation();
+                            e.preventDefault();
+                            handleAdd();
+                        }}
+                    >
+                        {error && <ErrorMessage>{error}</ErrorMessage>}
+                        <InputRow>
+                            <ScreenReaderOnly>
+                                <label htmlFor={inputId}>
+                                    Constraint Value
+                                </label>
+                            </ScreenReaderOnly>
+                            <StyledTextField
+                                id={inputId}
+                                placeholder='Enter value'
+                                value={inputValues}
+                                onChange={(e) => {
+                                    setInputValues(e.target.value);
+                                    setError('');
+                                }}
+                                size='small'
+                                variant='standard'
+                                fullWidth
+                                inputRef={inputRef}
+                                autoFocus
+                            />
+                            <Button
+                                variant='text'
+                                type='submit'
+                                color='primary'
+                                disabled={!inputValues.trim()}
+                            >
+                                Add
+                            </Button>
+                        </InputRow>
+                    </form>
+                </StyledPopover>
+            </>
+        );
+    },
+);

frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx

@@ -30,7 +30,7 @@ import type {
     IUnleashContextDefinition,
 } from 'interfaces/context';
 import type { IConstraint } from 'interfaces/strategy';
-import { useEffect, useState, type FC } from 'react';
+import { useEffect, useRef, useState, type FC } from 'react';
 import { oneOf } from 'utils/oneOf';
 import {
     CURRENT_TIME_CONTEXT_FIELD,
@@ -43,6 +43,7 @@ import { ValueList } from './ValueList';
 import { ReactComponent as CaseSensitiveIcon } from 'assets/icons/case-sensitive.svg';
 import { ReactComponent as CaseInsensitiveIcon } from 'assets/icons/case-insensitive.svg';
 import { ScreenReaderOnly } from 'component/common/ScreenReaderOnly/ScreenReaderOnly';
+import { AddValuesWidget } from './AddValuesWidget';
 
 const Container = styled('article')(({ theme }) => ({
     '--padding': theme.spacing(2),
@@ -143,6 +144,11 @@ const CaseButton = styled(StyledButton)(({ theme }) => ({
     placeItems: 'center',
 }));
 
+const OPERATORS_WITH_ADD_VALUES_WIDGET = [
+    'IN_OPERATORS_FREETEXT',
+    'STRING_OPERATORS_FREETEXT',
+];
+
 type Props = {
     localConstraint: IConstraint;
     setContextName: (contextName: string) => void;
@@ -190,6 +196,10 @@ export const EditableConstraint: FC<Props> = ({
     const { contextName, operator } = localConstraint;
     const [showCaseSensitiveButton, setShowCaseSensitiveButton] =
         useState(false);
+    const deleteButtonRef = useRef<HTMLButtonElement>(null);
+    const addValuesButtonRef = useRef<HTMLButtonElement>(null);
+    const showAddValuesButton =
+        OPERATORS_WITH_ADD_VALUES_WIDGET.includes(input);
 
     /* We need a special case to handle the currentTime context field. Since
     this field will be the only one to allow DATE_BEFORE and DATE_AFTER operators
@@ -410,17 +420,40 @@ export const EditableConstraint: FC<Props> = ({
                         </CaseButton>
                     ) : null}
 
-                    {!input.includes('LEGAL_VALUES') && (
                     <ValueList
                         values={localConstraint.values}
                         removeValue={removeValue}
                         setValues={setValuesWithRecord}
+                        getExternalFocusTarget={() =>
+                            addValuesButtonRef.current ??
+                            deleteButtonRef.current
+                        }
+                    >
+                        {showAddValuesButton ? (
+                            <AddValuesWidget
+                                ref={addValuesButtonRef}
+                                onAddValues={(newValues) => {
+                                    // todo (`addEditStrategy`): move deduplication logic higher up in the context handling
+                                    const combinedValues = new Set([
+                                        ...(localConstraint.values || []),
+                                        ...newValues,
+                                    ]);
+                                    setValuesWithRecord(
+                                        Array.from(combinedValues),
+                                    );
+                                }}
                             />
-                    )}
+                        ) : null}
+                    </ValueList>
                 </ConstraintDetails>
 
                 <HtmlTooltip title='Delete constraint' arrow>
-                    <IconButton type='button' size='small' onClick={onDelete}>
+                    <IconButton
+                        type='button'
+                        size='small'
+                        onClick={onDelete}
+                        ref={deleteButtonRef}
+                    >
                         <Delete />
                     </IconButton>
                 </HtmlTooltip>

frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/ValueList.tsx

@@ -1,23 +1,6 @@
-import Add from '@mui/icons-material/Add';
 import Clear from '@mui/icons-material/Clear';
-import {
-    Button,
-    Chip,
-    type ChipProps,
-    Popover,
-    styled,
-    TextField,
-} from '@mui/material';
-import { ScreenReaderOnly } from 'component/common/ScreenReaderOnly/ScreenReaderOnly';
-import {
-    type FC,
-    forwardRef,
-    useId,
-    useImperativeHandle,
-    useRef,
-    useState,
-} from 'react';
-import { parseParameterStrings } from 'utils/parseParameter';
+import { Chip, type ChipProps, styled } from '@mui/material';
+import { type FC, forwardRef, type PropsWithChildren, useRef } from 'react';
 
 const ValueListWrapper = styled('div')(({ theme }) => ({
     display: 'flex',
@@ -72,181 +55,28 @@ const ValueChip = styled(ValueChipBase)(({ theme }) => ({
     },
 }));
 
-const AddValuesButton = styled('button')(({ theme }) => ({
-    color: theme.palette.primary.main,
-    svg: {
-        fill: theme.palette.primary.main,
-        height: theme.fontSizes.smallerBody,
-        width: theme.fontSizes.smallerBody,
-    },
-    border: 'none',
-    borderRadius: theme.shape.borderRadiusExtraLarge,
-    display: 'flex',
-    flexFlow: 'row nowrap',
-    whiteSpace: 'nowrap',
-    gap: theme.spacing(0.25),
-    alignItems: 'center',
-    padding: theme.spacing(0.5, 1.5, 0.5, 1.5),
-    height: 'auto',
-    transition: 'all 0.3s ease',
-    outline: `1px solid #0000`,
-    background: theme.palette.background.elevation1,
-    ':hover, :focus-visible': {
-        background: theme.palette.background.elevation1,
-        outlineColor: theme.palette.secondary.dark,
-    },
-}));
-
-const StyledPopover = styled(Popover)(({ theme }) => ({
-    '& .MuiPaper-root': {
-        borderRadius: theme.shape.borderRadiusLarge,
-        border: `1px solid ${theme.palette.divider}`,
-        padding: theme.spacing(2),
-        width: '250px',
-    },
-}));
-
-const StyledTextField = styled(TextField)(({ theme }) => ({
-    flexGrow: 1,
-}));
-
-const InputRow = styled('div')(({ theme }) => ({
-    display: 'flex',
-    gap: theme.spacing(1),
-    alignItems: 'start',
-    width: '100%',
-}));
-
-const ErrorMessage = styled('div')(({ theme }) => ({
-    color: theme.palette.error.main,
-    fontSize: theme.typography.caption.fontSize,
-    marginBottom: theme.spacing(1),
-}));
-
-interface AddValuesProps {
-    onAddValues: (values: string[]) => void;
-}
-
-const AddValues = forwardRef<HTMLButtonElement, AddValuesProps>(
-    ({ onAddValues }, ref) => {
-        const [open, setOpen] = useState(false);
-        const [inputValues, setInputValues] = useState('');
-        const [error, setError] = useState('');
-        const positioningRef = useRef<HTMLButtonElement>(null);
-        useImperativeHandle(
-            ref,
-            () => positioningRef.current as HTMLButtonElement,
-        );
-        const inputRef = useRef<HTMLInputElement>(null);
-        const inputId = useId();
-
-        const handleAdd = () => {
-            const newValues = parseParameterStrings(inputValues);
-
-            if (newValues.length === 0) {
-                setError('Values cannot be empty');
-                return;
-            }
-
-            if (newValues.some((v) => v.length > 100)) {
-                setError('Values cannot be longer than 100 characters');
-                return;
-            }
-
-            onAddValues(newValues);
-            setInputValues('');
-            setError('');
-            inputRef?.current?.focus();
-        };
-
-        return (
-            <>
-                <AddValuesButton
-                    ref={positioningRef}
-                    onClick={() => setOpen(true)}
-                    type='button'
-                >
-                    <Add />
-                    <span>Add values</span>
-                </AddValuesButton>
-                <StyledPopover
-                    open={open}
-                    disableScrollLock
-                    anchorEl={positioningRef.current}
-                    onClose={() => setOpen(false)}
-                    anchorOrigin={{
-                        vertical: 'bottom',
-                        horizontal: 'left',
-                    }}
-                    transformOrigin={{
-                        vertical: 'top',
-                        horizontal: 'left',
-                    }}
-                >
-                    <form
-                        onSubmit={(e) => {
-                            e.stopPropagation();
-                            e.preventDefault();
-                            handleAdd();
-                        }}
-                    >
-                        {error && <ErrorMessage>{error}</ErrorMessage>}
-                        <InputRow>
-                            <ScreenReaderOnly>
-                                <label htmlFor={inputId}>
-                                    Constraint Value
-                                </label>
-                            </ScreenReaderOnly>
-                            <StyledTextField
-                                id={inputId}
-                                placeholder='Enter value'
-                                value={inputValues}
-                                onChange={(e) => {
-                                    setInputValues(e.target.value);
-                                    setError('');
-                                }}
-                                size='small'
-                                variant='standard'
-                                fullWidth
-                                inputRef={inputRef}
-                                autoFocus
-                            />
-                            <Button
-                                variant='text'
-                                type='submit'
-                                color='primary'
-                                disabled={!inputValues.trim()}
-                            >
-                                Add
-                            </Button>
-                        </InputRow>
-                    </form>
-                </StyledPopover>
-            </>
-        );
-    },
-);
-
 type Props = {
     values: string[] | undefined;
     removeValue: (index: number) => void;
     setValues: (values: string[]) => void;
+    // the element that should receive focus when all value chips are deleted
+    getExternalFocusTarget: () => HTMLElement | null;
 };
 
-export const ValueList: FC<Props> = ({
+export const ValueList: FC<PropsWithChildren<Props>> = ({
     values = [],
     removeValue,
-    setValues,
+    getExternalFocusTarget,
+    children,
 }) => {
     const constraintElementRefs: React.MutableRefObject<
         (HTMLDivElement | null)[]
     > = useRef([]);
-    const addValuesButtonRef = useRef<HTMLButtonElement>(null);
 
     const nextFocusTarget = (deletedIndex: number) => {
         if (deletedIndex === values.length - 1) {
             if (deletedIndex === 0) {
-                return addValuesButtonRef.current;
+                return getExternalFocusTarget();
             } else {
                 return constraintElementRefs.current[deletedIndex - 1];
             }
@@ -255,11 +85,6 @@ export const ValueList: FC<Props> = ({
         }
     };
 
-    const handleAddValues = (newValues: string[]) => {
-        const combinedValues = uniqueValues([...(values || []), ...newValues]);
-        setValues(combinedValues);
-    };
-
     return (
         <ValueListWrapper>
             <StyledList>
@@ -279,11 +104,7 @@ export const ValueList: FC<Props> = ({
                     </li>
                 ))}
             </StyledList>
-            <AddValues ref={addValuesButtonRef} onAddValues={handleAddValues} />
+            {children}
         </ValueListWrapper>
     );
 };
-
-const uniqueValues = <T,>(values: T[]): T[] => {
-    return Array.from(new Set(values));
-};