From 44082b24a1c932578a5c24341bccd0d8b481b9d2 Mon Sep 17 00:00:00 2001
From: Thomas Heartman <thomas@getunleash.io>
Date: Fri, 25 Apr 2025 12:37:04 +0200
Subject: [PATCH] chore: second design pass for editable constraints (#9843)

Fix a number of visual issues with the main editable constraint
component.

I've introduced a few more layers of container nesting to make the
layout break the right ways:
- Put everything on the same line when on wide.
- At 700px place selected values on the row below
- From 700px down, when necessary, also wrap operator options


Support super long context names without breaking layout
<img width="399" alt="image"
src="https://github.com/user-attachments/assets/07555e9c-d875-417f-ae6b-d4600731d5eb"
/>

Wrap values at 700px width container:
<img width="703" alt="image"
src="https://github.com/user-attachments/assets/deb6e059-57d4-4e47-88da-3ec5d6bce751"
/>

Wrap operator options when necessary
<img width="359" alt="image"
src="https://github.com/user-attachments/assets/ff96db40-f47d-4ddf-bed7-dfced4d69973"
/>

Absolutely position delete button to allow to not push it out of the
container on narrow screens:
<img width="330" alt="image"
src="https://github.com/user-attachments/assets/c7b8f88d-538a-46a1-ae3f-e5a761b50289"
/>

Remove extra focus styling from MUI (darken select background):
Before:
<img width="348" alt="image"
src="https://github.com/user-attachments/assets/99aff08d-c1af-46c0-8a75-40c1ea3c103f"
/>

<img width="357" alt="image"
src="https://github.com/user-attachments/assets/b7a0edac-2716-48a7-b50c-b3437e5f5be8"
/>

After:
<img width="379" alt="image"
src="https://github.com/user-attachments/assets/74da884c-7b1a-4b9a-8383-31592326a71b"
/>
<img width="350" alt="image"
src="https://github.com/user-attachments/assets/0ebea696-5f7d-4d4e-b91c-b087a8fc56a3"
/>
---
 .../ConstraintOperatorSelect.tsx              |   3 +
 .../EditableConstraint.tsx                    | 144 ++++++++++++------
 .../FeatureStrategyConstraints/ValueList.tsx  |   7 +
 3 files changed, 105 insertions(+), 49 deletions(-)

diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/ConstraintOperatorSelect.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/ConstraintOperatorSelect.tsx
index 0dbe8f134a..3f54d319ed 100644
--- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/ConstraintOperatorSelect.tsx
+++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/ConstraintOperatorSelect.tsx
@@ -47,6 +47,9 @@ const StyledSelect = styled(Select)(({ theme }) => ({
     '.MuiInput-input': {
         paddingBlock: theme.spacing(0.25),
     },
+    ':focus-within .MuiSelect-select': {
+        background: 'none',
+    },
 }));
 
 const StyledMenuItem = styled(MenuItem, {
diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx
index 5b3564726f..e3264fa76a 100644
--- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx
+++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx
@@ -32,22 +32,44 @@ const Container = styled('article')(({ theme }) => ({
     backgroundColor: theme.palette.background.paper,
     borderRadius: theme.shape.borderRadiusLarge,
     border: `1px solid ${theme.palette.divider}`,
+    containerType: 'inline-size',
 }));
 
+const onNarrowContainer = '@container (max-width: 700px)';
+
 const TopRow = styled('div')(({ theme }) => ({
+    '--gap': theme.spacing(1),
     padding: 'var(--padding)',
     display: 'flex',
     flexFlow: 'row nowrap',
     alignItems: 'flex-start',
     justifyItems: 'space-between',
+    gap: 'var(--gap)',
+}));
+
+const ConstraintOptions = styled('div')(({ theme }) => ({
+    display: 'flex',
+    flexFlow: 'row nowrap',
+    gap: 'var(--gap)',
+    alignSelf: 'flex-start',
+    [onNarrowContainer]: {
+        flexFlow: 'row wrap',
+    },
+}));
+
+const OperatorOptions = styled(ConstraintOptions)(({ theme }) => ({
+    flexFlow: 'row nowrap',
 }));
 
 const ConstraintDetails = styled('div')(({ theme }) => ({
     display: 'flex',
-    gap: theme.spacing(1),
+    gap: 'var(--gap)',
     flexFlow: 'row nowrap',
     width: '100%',
     height: 'min-content',
+    [onNarrowContainer]: {
+        flexDirection: 'column',
+    },
 }));
 
 const InputContainer = styled('div')(({ theme }) => ({
@@ -57,6 +79,10 @@ const InputContainer = styled('div')(({ theme }) => ({
 
 const StyledSelect = styled(GeneralSelect)(({ theme }) => ({
     fieldset: { border: 'none', borderRadius: 0 },
+    maxWidth: '25ch',
+    ':focus-within .MuiSelect-select': {
+        background: 'none',
+    },
     ':focus-within fieldset': { borderBottomStyle: 'solid' },
     'label + &': {
         // mui adds a margin top to 'standard' selects with labels
@@ -67,10 +93,19 @@ const StyledSelect = styled(GeneralSelect)(({ theme }) => ({
     },
 }));
 
+const StyledIconButton = styled(IconButton)(({ theme }) => ({
+    position: 'absolute',
+    right: theme.spacing(1),
+}));
+
 const StyledButton = styled('button')(({ theme }) => ({
+    // todo (`addEditStrategy`): this is pretty rough, but it needs to be the
+    // same height as the input fields, which are 27.25 px at the moment.
+    // Consider editing this when we get new icons for the buttons. There may be
+    // a better solution.
+    height: `calc(${theme.typography.body1.fontSize} + ${theme.spacing(1.5)})`,
     width: '5ch',
     borderRadius: theme.shape.borderRadius,
-    padding: theme.spacing(0.25, 0),
     fontSize: theme.fontSizes.smallerBody,
     background: theme.palette.secondary.light,
     border: `1px solid ${theme.palette.secondary.border}`,
@@ -82,6 +117,14 @@ const StyledButton = styled('button')(({ theme }) => ({
     },
 }));
 
+const ButtonPlaceholder = styled('div')(({ theme }) => ({
+    // this is a trick that lets us use absolute positioning for the button so
+    // that it can go over the operator context fields when necessary (narrow
+    // screens), but still retain necessary space for the button when it's all
+    // on one line.
+    width: theme.spacing(2),
+}));
+
 const StyledCaseInsensitiveIcon = styled(CaseInsensitiveIcon)(({ theme }) => ({
     path: {
         fill: theme.palette.text.disabled,
@@ -218,52 +261,55 @@ export const EditableConstraint: FC<Props> = ({
         <Container>
             <TopRow>
                 <ConstraintDetails>
-                    <StyledSelect
-                        visuallyHideLabel
-                        id='context-field-select'
-                        name='contextName'
-                        label='Context Field'
-                        autoFocus
-                        options={constraintNameOptions}
-                        value={contextName || ''}
-                        onChange={setContextName}
-                        variant='standard'
-                    />
+                    <ConstraintOptions>
+                        <StyledSelect
+                            visuallyHideLabel
+                            id='context-field-select'
+                            name='contextName'
+                            label='Context Field'
+                            autoFocus
+                            options={constraintNameOptions}
+                            value={contextName || ''}
+                            onChange={setContextName}
+                            variant='standard'
+                        />
 
-                    <StyledButton
-                        type='button'
-                        onClick={toggleInvertedOperator}
-                    >
-                        {localConstraint.inverted ? 'aint' : 'is'}
-                    </StyledButton>
+                        <OperatorOptions>
+                            <StyledButton
+                                type='button'
+                                onClick={toggleInvertedOperator}
+                            >
+                                {localConstraint.inverted ? 'aint' : 'is'}
+                            </StyledButton>
 
-                    <ConstraintOperatorSelect
-                        options={operatorsForContext(contextName)}
-                        value={operator}
-                        onChange={onOperatorChange}
-                        inverted={localConstraint.inverted}
-                    />
-
-                    {showCaseSensitiveButton ? (
-                        <CaseButton
-                            type='button'
-                            onClick={toggleCaseSensitivity}
-                        >
-                            {localConstraint.caseInsensitive ? (
-                                <StyledCaseInsensitiveIcon aria-label='The match is not case sensitive.' />
-                            ) : (
-                                <StyledCaseSensitiveIcon aria-label='The match is case sensitive.' />
-                            )}
-                            <ScreenReaderOnly>
-                                Make match
-                                {localConstraint.caseInsensitive
-                                    ? ' '
-                                    : ' not '}
-                                case sensitive
-                            </ScreenReaderOnly>
-                        </CaseButton>
-                    ) : null}
+                            <ConstraintOperatorSelect
+                                options={operatorsForContext(contextName)}
+                                value={operator}
+                                onChange={onOperatorChange}
+                                inverted={localConstraint.inverted}
+                            />
 
+                            {showCaseSensitiveButton ? (
+                                <CaseButton
+                                    type='button'
+                                    onClick={toggleCaseSensitivity}
+                                >
+                                    {localConstraint.caseInsensitive ? (
+                                        <StyledCaseInsensitiveIcon aria-label='The match is not case sensitive.' />
+                                    ) : (
+                                        <StyledCaseSensitiveIcon aria-label='The match is case sensitive.' />
+                                    )}
+                                    <ScreenReaderOnly>
+                                        Make match
+                                        {localConstraint.caseInsensitive
+                                            ? ' '
+                                            : ' not '}
+                                        case sensitive
+                                    </ScreenReaderOnly>
+                                </CaseButton>
+                            ) : null}
+                        </OperatorOptions>
+                    </ConstraintOptions>
                     <ValueList
                         values={localConstraint.values}
                         removeValue={removeValue}
@@ -290,16 +336,16 @@ export const EditableConstraint: FC<Props> = ({
                         ) : null}
                     </ValueList>
                 </ConstraintDetails>
-
+                <ButtonPlaceholder />
                 <HtmlTooltip title='Delete constraint' arrow>
-                    <IconButton
+                    <StyledIconButton
                         type='button'
                         size='small'
                         onClick={onDelete}
                         ref={deleteButtonRef}
                     >
-                        <Delete />
-                    </IconButton>
+                        <Delete fontSize='inherit' />
+                    </StyledIconButton>
                 </HtmlTooltip>
             </TopRow>
             {showInputField ? (
diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/ValueList.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/ValueList.tsx
index 883c00bc9e..10bb1c9d08 100644
--- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/ValueList.tsx
+++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/ValueList.tsx
@@ -94,6 +94,13 @@ export const ValueList: FC<PropsWithChildren<Props>> = ({
                             ref={(el) => {
                                 constraintElementRefs.current[index] = el;
                             }}
+                            sx={{
+                                height: 'auto',
+                                '& .MuiChip-label': {
+                                    display: 'block',
+                                    whiteSpace: 'normal',
+                                },
+                            }}
                             deleteIcon={<Clear />}
                             label={value}
                             onDelete={() => {