fix: proper spacing and dividers between strategies (#1187)

* fix: proper spacing and dividers between strategies

* fix: improve strategy execution list logic

* update custom strategy execution styles

* interpret not defined custom strategy parameters

frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.styles.ts

@@ -2,17 +2,11 @@ import { makeStyles } from 'tss-react/mui';
 
 export const useStyles = makeStyles()(theme => ({
     valueContainer: {
-        display: 'flex',
+        padding: theme.spacing(2, 3),
-        alignItems: 'center',
+        border: `1px solid ${theme.palette.dividerAlternative}`,
-        gap: '1ch',
+        borderRadius: theme.shape.borderRadius,
     },
     valueSeparator: {
         color: theme.palette.grey[700],
     },
-    summary: {
-        width: '100%',
-        padding: theme.spacing(2, 3),
-        borderRadius: theme.shape.borderRadius,
-        border: `1px solid ${theme.palette.divider}`,
-    },
 }));

frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.tsx

@@ -1,56 +1,62 @@
-import { Fragment } from 'react';
+import { Fragment, useMemo, VFC } from 'react';
-import { Box, Chip } from '@mui/material';
+import { Box, Chip, Tooltip } from '@mui/material';
 import { IFeatureStrategy } from 'interfaces/strategy';
 import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
 import PercentageCircle from 'component/common/PercentageCircle/PercentageCircle';
 import { StrategySeparator } from 'component/common/StrategySeparator/StrategySeparator';
 import { ConstraintItem } from './ConstraintItem/ConstraintItem';
 import { useStrategies } from 'hooks/api/getters/useStrategies/useStrategies';
-import StringTruncator from 'component/common/StringTruncator/StringTruncator';
+import { useSegments } from 'hooks/api/getters/useSegments/useSegments';
 import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';
 import { FeatureOverviewSegment } from 'component/feature/FeatureView/FeatureOverview/FeatureOverviewSegment/FeatureOverviewSegment';
 import { ConstraintAccordionList } from 'component/common/ConstraintAccordion/ConstraintAccordionList/ConstraintAccordionList';
 import { useStyles } from './StrategyExecution.styles';
 import {
-    parseParameterString,
     parseParameterNumber,
+    parseParameterString,
     parseParameterStrings,
 } from 'utils/parseParameter';
+import StringTruncator from 'component/common/StringTruncator/StringTruncator';
 
 interface IStrategyExecutionProps {
     strategy: IFeatureStrategy;
     percentageFill?: string;
 }
 
-export const StrategyExecution = ({ strategy }: IStrategyExecutionProps) => {
+const NoItems: VFC = () => (
+    <Box sx={{ px: 3, color: 'text.disabled' }}>
+        This strategy does not have constraints or parameters.
+    </Box>
+);
+
+export const StrategyExecution: VFC<IStrategyExecutionProps> = ({
+    strategy,
+}) => {
     const { parameters, constraints = [] } = strategy;
     const { classes: styles } = useStyles();
     const { strategies } = useStrategies();
     const { uiConfig } = useUiConfig();
-
+    const { segments } = useSegments(strategy.id);
-    if (!parameters) {
-        return null;
-    }
 
     const definition = strategies.find(strategyDefinition => {
         return strategyDefinition.name === strategy.name;
     });
 
-    const renderParameters = () => {
+    const parametersList = useMemo(() => {
-        if (definition?.editable) return null;
+        if (!parameters || definition?.editable) return null;
 
         return Object.keys(parameters).map(key => {
             switch (key) {
                 case 'rollout':
                 case 'Rollout':
                     const percentage = parseParameterNumber(parameters[key]);
+
                     return (
                         <Box
-                            className={styles.summary}
+                            className={styles.valueContainer}
-                            key={key}
                             sx={{ display: 'flex', alignItems: 'center' }}
                         >
-                            <Box sx={{ mr: '1rem' }}>
+                            <Box sx={{ mr: 2 }}>
                                 <PercentageCircle
                                     percentage={percentage}
                                     size="2rem"
@@ -93,178 +99,163 @@ export const StrategyExecution = ({ strategy }: IStrategyExecutionProps) => {
                     return null;
             }
         });
-    };
+    }, [parameters, definition, constraints, styles]);
+
+    const customStrategyList = useMemo(() => {
+        if (!parameters || !definition?.editable) return null;
+        const isSetTo = (
+            <span className={styles.valueSeparator}>{' is set to '}</span>
+        );
+
+        return definition?.parameters.map(param => {
+            const { type, name } = { ...param };
+            if (!type || !name || parameters[name] === undefined) {
+                return null;
+            }
+            const nameItem = (
+                <StringTruncator maxLength={15} maxWidth="150" text={name} />
+            );
 
-    const renderCustomStrategy = () => {
-        if (!definition?.editable) return null;
-        return definition?.parameters.map((param: any, index: number) => {
-            const notLastItem = index !== definition?.parameters?.length - 1;
             switch (param?.type) {
                 case 'list':
-                    const values = parseParameterStrings(
+                    const values = parseParameterStrings(parameters[name]);
-                        strategy?.parameters[param.name]
+
-                    );
+                    return values.length > 0 ? (
-                    return (
+                        <div className={styles.valueContainer}>
-                        <Fragment key={param?.name}>
+                            {nameItem}{' '}
-                            <ConstraintItem value={values} text={param.name} />
-                            <ConditionallyRender
-                                condition={notLastItem}
-                                show={<StrategySeparator text="AND" />}
-                            />
-                        </Fragment>
-                    );
-                case 'percentage':
-                    return (
-                        <Fragment key={param?.name}>
-                            <div>
-                                <Chip
-                                    size="small"
-                                    variant="outlined"
-                                    color="success"
-                                    label={`${
-                                        strategy?.parameters[param.name]
-                                    }%`}
-                                />{' '}
-                                of your base{' '}
-                                {constraints?.length > 0
-                                    ? 'who match constraints'
-                                    : ''}{' '}
-                                is included.
-                            </div>
-                            <PercentageCircle
-                                percentage={parseParameterNumber(
-                                    strategy.parameters[param.name]
-                                )}
-                            />
-                            <ConditionallyRender
-                                condition={notLastItem}
-                                show={<StrategySeparator text="AND" />}
-                            />
-                        </Fragment>
-                    );
-                case 'boolean':
-                    return (
-                        <Fragment key={param.name}>
-                            <p key={param.name}>
-                                <StringTruncator
-                                    maxLength={15}
-                                    maxWidth="150"
-                                    text={param.name}
-                                />{' '}
-                                {strategy.parameters[param.name]}
-                            </p>
-                            <ConditionallyRender
-                                condition={
-                                    typeof strategy.parameters[param.name] !==
-                                    'undefined'
-                                }
-                                show={
-                                    <ConditionallyRender
-                                        condition={notLastItem}
-                                        show={<StrategySeparator text="AND" />}
-                                    />
-                                }
-                            />
-                        </Fragment>
-                    );
-                case 'string':
-                    const value = parseParameterString(
-                        strategy.parameters[param.name]
-                    );
-                    return (
-                        <ConditionallyRender
-                            condition={
-                                typeof strategy.parameters[param.name] !==
-                                'undefined'
-                            }
-                            key={param.name}
-                            show={
-                                <>
-                                    <p className={styles.valueContainer}>
-                                        <StringTruncator
-                                            maxWidth="150"
-                                            maxLength={15}
-                                            text={param.name}
-                                        />
                             <span className={styles.valueSeparator}>
-                                            is set to
+                                has {values.length}{' '}
+                                {values.length > 1 ? `items` : 'item'}:{' '}
+                                {values.map((item: string) => (
+                                    <Chip
+                                        key={item}
+                                        label={
+                                            <StringTruncator
+                                                maxWidth="300"
+                                                text={item}
+                                                maxLength={50}
+                                            />
+                                        }
+                                        sx={{ mr: 0.5 }}
+                                    />
+                                ))}
                             </span>
+                        </div>
+                    ) : null;
+
+                case 'percentage':
+                    const percentage = parseParameterNumber(parameters[name]);
+                    return parameters[name] !== '' ? (
+                        <Box
+                            className={styles.valueContainer}
+                            sx={{ display: 'flex', alignItems: 'center' }}
+                        >
+                            <Box sx={{ mr: 2 }}>
+                                <PercentageCircle
+                                    percentage={percentage}
+                                    size="2rem"
+                                />
+                            </Box>
+                            <div>
+                                {nameItem}
+                                {isSetTo}
+                                <Chip
+                                    color="success"
+                                    variant="outlined"
+                                    size="small"
+                                    label={`${percentage}%`}
+                                />
+                            </div>
+                        </Box>
+                    ) : null;
+
+                case 'boolean':
+                    return parameters[name] === 'true' ||
+                        parameters[name] === 'false' ? (
+                        <div className={styles.valueContainer}>
+                            <StringTruncator
+                                maxLength={15}
+                                maxWidth="150"
+                                text={name}
+                            />
+                            {isSetTo}
+                            <Chip
+                                color={
+                                    parameters[name] === 'true'
+                                        ? 'success'
+                                        : 'error'
+                                }
+                                variant="outlined"
+                                size="small"
+                                label={parameters[name]}
+                            />
+                        </div>
+                    ) : null;
+
+                case 'string':
+                    const value = parseParameterString(parameters[name]);
+                    return typeof parameters[name] !== 'undefined' ? (
+                        <div className={styles.valueContainer}>
+                            {nameItem}
+                            <ConditionallyRender
+                                condition={value === ''}
+                                show={
+                                    <span className={styles.valueSeparator}>
+                                        {' is an empty string'}
+                                    </span>
+                                }
+                                elseShow={
+                                    <>
+                                        {isSetTo}
                                         <StringTruncator
                                             maxWidth="300"
                                             text={value}
                                             maxLength={50}
                                         />
-                                    </p>
-                                    <ConditionallyRender
-                                        condition={notLastItem}
-                                        show={<StrategySeparator text="AND" />}
-                                    />
                                     </>
                                 }
                             />
-                    );
+                        </div>
+                    ) : null;
+
                 case 'number':
-                    const number = parseParameterNumber(
+                    const number = parseParameterNumber(parameters[name]);
-                        strategy.parameters[param.name]
+                    return parameters[name] !== '' && number !== undefined ? (
-                    );
+                        <div className={styles.valueContainer}>
-                    return (
+                            {nameItem}
-                        <ConditionallyRender
+                            {isSetTo}
-                            condition={number !== undefined}
-                            key={param.name}
-                            show={
-                                <>
-                                    <p className={styles.valueContainer}>
-                                        <StringTruncator
-                                            maxLength={15}
-                                            maxWidth="150"
-                                            text={param.name}
-                                        />
-                                        <span className={styles.valueSeparator}>
-                                            is set to
-                                        </span>
                             <StringTruncator
                                 maxWidth="300"
                                 text={String(number)}
                                 maxLength={50}
                             />
-                                    </p>
+                        </div>
-                                    <ConditionallyRender
+                    ) : null;
-                                        condition={notLastItem}
-                                        show={<StrategySeparator text="AND" />}
-                                    />
-                                </>
-                            }
-                        />
-                    );
                 case 'default':
                     return null;
             }
+
             return null;
         });
-    };
+    }, [parameters, definition, styles]);
 
-    return (
+    if (!parameters) {
-        <>
+        return <NoItems />;
-            <ConditionallyRender
+    }
-                condition={Boolean(uiConfig.flags.SE)}
+
-                show={<FeatureOverviewSegment strategyId={strategy.id} />}
+    const listItems = [
-            />
+        Boolean(uiConfig.flags.SE) && segments && segments.length > 0 && (
-            <ConditionallyRender
+            <FeatureOverviewSegment strategyId={strategy.id} />
-                condition={constraints.length > 0}
+        ),
-                show={
+        constraints.length > 0 && (
-                    <>
             <ConstraintAccordionList
                 constraints={constraints}
                 showLabel={false}
             />
-                        <StrategySeparator text="AND" />
+        ),
-                    </>
+        strategy.name === 'default' && (
-                }
+            <>
-            />
+                <Box sx={{ width: '100%' }} className={styles.valueContainer}>
-            <ConditionallyRender
-                condition={strategy.name === 'default'}
-                show={
-                    <Box sx={{ width: '100%' }} className={styles.summary}>
                     The standard strategy is{' '}
                     <Chip
                         variant="outlined"
@@ -274,10 +265,29 @@ export const StrategyExecution = ({ strategy }: IStrategyExecutionProps) => {
                     />{' '}
                     for all users.
                 </Box>
-                }
-            />
-            {renderParameters()}
-            {renderCustomStrategy()}
             </>
+        ),
+        ...(parametersList ?? []),
+        ...(customStrategyList ?? []),
+    ].filter(Boolean);
+
+    return (
+        <ConditionallyRender
+            condition={listItems.length > 0}
+            show={
+                <>
+                    {listItems.map((item, index) => (
+                        <Fragment key={index}>
+                            <ConditionallyRender
+                                condition={index > 0}
+                                show={<StrategySeparator text="AND" />}
+                            />
+                            {item}
+                        </Fragment>
+                    ))}
+                </>
+            }
+            elseShow={<NoItems />}
+        />
     );
 };

frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSegment/FeatureOverviewSegment.tsx

@@ -3,7 +3,6 @@ import { Link } from 'react-router-dom';
 import { DonutLarge } from '@mui/icons-material';
 import { useStyles } from 'component/feature/FeatureView/FeatureOverview/FeatureOverviewSegment/FeatureOverviewSegment.styles';
 import { useSegments } from 'hooks/api/getters/useSegments/useSegments';
-import { StrategySeparator } from 'component/common/StrategySeparator/StrategySeparator';
 
 interface IFeatureOverviewSegmentProps {
     strategyId: string;
@@ -32,7 +31,6 @@ export const FeatureOverviewSegment = ({
                             {segment.name}
                         </Link>
                     </div>
-                    <StrategySeparator text="AND" />
                 </Fragment>
             ))}
         </>

frontend/src/themes/theme.ts

@@ -309,6 +309,11 @@ export default createTheme({
                             ...(ownerState.color === 'default' && {
                                 color: theme.palette.text.secondary,
                             }),
+                            ...(ownerState.color === 'error' && {
+                                color: theme.palette.error.dark,
+                                background: theme.palette.error.light,
+                                borderColor: theme.palette.error.border,
+                            }),
                         }),
                 }),
             },