From 537bcdc1b710128eb58af3c5e4ec392d40f8235f Mon Sep 17 00:00:00 2001 From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com> Date: Wed, 3 Aug 2022 09:23:57 +0200 Subject: [PATCH] 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 --- .../StrategyExecution.styles.ts | 12 +- .../StrategyExecution/StrategyExecution.tsx | 372 +++++++++--------- .../FeatureOverviewSegment.tsx | 2 - frontend/src/themes/theme.ts | 5 + 4 files changed, 199 insertions(+), 192 deletions(-) diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.styles.ts b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.styles.ts index e2d49389e1..d1fa29e035 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.styles.ts +++ b/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', - alignItems: 'center', - gap: '1ch', + padding: theme.spacing(2, 3), + border: `1px solid ${theme.palette.dividerAlternative}`, + 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}`, - }, })); diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.tsx index 7618d0fae4..99efbdd766 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.tsx @@ -1,56 +1,62 @@ -import { Fragment } from 'react'; -import { Box, Chip } from '@mui/material'; +import { Fragment, useMemo, VFC } from 'react'; +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 = () => ( + + This strategy does not have constraints or parameters. + +); + +export const StrategyExecution: VFC = ({ + strategy, +}) => { const { parameters, constraints = [] } = strategy; const { classes: styles } = useStyles(); const { strategies } = useStrategies(); const { uiConfig } = useUiConfig(); - - if (!parameters) { - return null; - } + const { segments } = useSegments(strategy.id); const definition = strategies.find(strategyDefinition => { return strategyDefinition.name === strategy.name; }); - const renderParameters = () => { - if (definition?.editable) return null; + const parametersList = useMemo(() => { + if (!parameters || definition?.editable) return null; return Object.keys(parameters).map(key => { switch (key) { case 'rollout': case 'Rollout': const percentage = parseParameterNumber(parameters[key]); + return ( - + { return null; } }); - }; + }, [parameters, definition, constraints, styles]); + + const customStrategyList = useMemo(() => { + if (!parameters || !definition?.editable) return null; + const isSetTo = ( + {' is set to '} + ); + + return definition?.parameters.map(param => { + const { type, name } = { ...param }; + if (!type || !name || parameters[name] === undefined) { + return null; + } + const nameItem = ( + + ); - 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( - strategy?.parameters[param.name] - ); - return ( - - - } - /> - - ); - case 'percentage': - return ( - -
- {' '} - of your base{' '} - {constraints?.length > 0 - ? 'who match constraints' - : ''}{' '} - is included. -
- - } - /> - - ); - case 'boolean': - return ( - -

- {' '} - {strategy.parameters[param.name]} -

- } + const values = parseParameterStrings(parameters[name]); + + return values.length > 0 ? ( +
+ {nameItem}{' '} + + has {values.length}{' '} + {values.length > 1 ? `items` : 'item'}:{' '} + {values.map((item: string) => ( + + } + sx={{ mr: 0.5 }} /> - } + ))} + +
+ ) : null; + + case 'percentage': + const percentage = parseParameterNumber(parameters[name]); + return parameters[name] !== '' ? ( + + + + +
+ {nameItem} + {isSetTo} + +
+ + ) : null; + + case 'boolean': + return parameters[name] === 'true' || + parameters[name] === 'false' ? ( +
+ - - ); + {isSetTo} + +
+ ) : null; + case 'string': - const value = parseParameterString( - strategy.parameters[param.name] - ); - return ( - -

- - - is set to - + const value = parseParameterString(parameters[name]); + return typeof parameters[name] !== 'undefined' ? ( +

+ {nameItem} + + {' is an empty string'} + + } + elseShow={ + <> + {isSetTo} -

- } - /> - - } - /> - ); + + } + /> +
+ ) : null; + case 'number': - const number = parseParameterNumber( - strategy.parameters[param.name] - ); - return ( - -

- - - is set to - - -

- } - /> - - } - /> - ); + const number = parseParameterNumber(parameters[name]); + return parameters[name] !== '' && number !== undefined ? ( +
+ {nameItem} + {isSetTo} + +
+ ) : null; case 'default': return null; } + return null; }); - }; + }, [parameters, definition, styles]); + + if (!parameters) { + return ; + } + + const listItems = [ + Boolean(uiConfig.flags.SE) && segments && segments.length > 0 && ( + + ), + constraints.length > 0 && ( + + ), + strategy.name === 'default' && ( + <> + + The standard strategy is{' '} + {' '} + for all users. + + + ), + ...(parametersList ?? []), + ...(customStrategyList ?? []), + ].filter(Boolean); return ( - <> - } - /> - 0} - show={ - <> - - - - } - /> - - The standard strategy is{' '} - {' '} - for all users. - - } - /> - {renderParameters()} - {renderCustomStrategy()} - + 0} + show={ + <> + {listItems.map((item, index) => ( + + 0} + show={} + /> + {item} + + ))} + + } + elseShow={} + /> ); }; diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSegment/FeatureOverviewSegment.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSegment/FeatureOverviewSegment.tsx index 8467f78c49..d81dcb8bdd 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSegment/FeatureOverviewSegment.tsx +++ b/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} - ))} diff --git a/frontend/src/themes/theme.ts b/frontend/src/themes/theme.ts index 14558c5061..695d4f5cfa 100644 --- a/frontend/src/themes/theme.ts +++ b/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, + }), }), }), },