From a064672635d0ce749535e246e5ee2dfc95b989fc Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Tue, 11 Mar 2025 11:36:14 +0100 Subject: [PATCH] chore(1-3422): playground strategies list (#9504) Initial rough work on adapting the playground strategies to the new designs. This PR primarily splits components into Legacy files and adds new replacements. There are *some* updates (including spacing and text color), but nothing juicy yet. However, I wanted to get this in now, before this PR grows even bigger. --- .../FeatureDetails/FeatureDetails.test.tsx | 34 +++- .../FeatureDetails/FeatureDetails.tsx | 107 ++++------ .../FeatureDetails/LegacyFeatureDetails.tsx | 184 ++++++++++++++++++ .../FeatureResultInfoPopoverCell.tsx | 42 ++-- ...cyPlaygroundResultFeatureStrategyList.tsx} | 0 ...aygroundResultFeatureStrategyList.test.tsx | 18 +- .../PlaygroundResultsFeatureStrategyList.tsx | 67 +++++++ .../PlaygroundResultChip.tsx | 11 +- 8 files changed, 373 insertions(+), 90 deletions(-) create mode 100644 frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureDetails/LegacyFeatureDetails.tsx rename frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/{PlaygroundResultFeatureStrategyList.tsx => LegacyPlaygroundResultFeatureStrategyList.tsx} (100%) create mode 100644 frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultsFeatureStrategyList.tsx diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureDetails/FeatureDetails.test.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureDetails/FeatureDetails.test.tsx index 00d35947cf..ed4ee5bcfe 100644 --- a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureDetails/FeatureDetails.test.tsx +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureDetails/FeatureDetails.test.tsx @@ -1,7 +1,8 @@ import { screen } from '@testing-library/react'; import { render } from 'utils/testRenderer'; -import { FeatureDetails } from './FeatureDetails'; +import { FeatureDetails as LegacyFeatureDetails } from './LegacyFeatureDetails'; import type { PlaygroundFeatureSchema, PlaygroundRequestSchema } from 'openapi'; +import { FeatureDetails } from './FeatureDetails'; const testCases = [ { @@ -10,7 +11,7 @@ const testCases = [ hasUnsatisfiedDependency: true, isEnabledInCurrentEnvironment: false, } as PlaygroundFeatureSchema, - expectedText1: 'This feature flag is False in development because', + expectedText1: /This feature flag is False in development because/, expectedText2: 'parent dependency is not satisfied and the environment is disabled', }, @@ -20,7 +21,7 @@ const testCases = [ hasUnsatisfiedDependency: true, isEnabledInCurrentEnvironment: true, } as PlaygroundFeatureSchema, - expectedText1: 'This feature flag is False in development because', + expectedText1: /This feature flag is False in development because/, expectedText2: 'parent dependency is not satisfied', }, { @@ -29,7 +30,7 @@ const testCases = [ hasUnsatisfiedDependency: false, isEnabledInCurrentEnvironment: false, } as PlaygroundFeatureSchema, - expectedText1: 'This feature flag is False in development because', + expectedText1: /This feature flag is False in development because/, expectedText2: 'the environment is disabled', }, { @@ -37,7 +38,7 @@ const testCases = [ feature: { isEnabled: true, } as PlaygroundFeatureSchema, - expectedText1: 'This feature flag is True in development because', + expectedText1: /This feature flag is True in development because/, expectedText2: 'at least one strategy is True', }, { @@ -48,7 +49,7 @@ const testCases = [ data: [{ name: 'custom' }], }, } as PlaygroundFeatureSchema, - expectedText1: 'This feature flag is Unknown in development because', + expectedText1: /This feature flag is Unknown in development because/, expectedText2: 'no strategies could be fully evaluated', }, { @@ -59,7 +60,7 @@ const testCases = [ data: [{ name: 'custom' }, { name: 'default' }], }, } as PlaygroundFeatureSchema, - expectedText1: 'This feature flag is Unknown in development because', + expectedText1: /This feature flag is Unknown in development because/, expectedText2: 'not all strategies could be fully evaluated', }, { @@ -70,12 +71,29 @@ const testCases = [ data: [{ name: 'default' }], }, } as PlaygroundFeatureSchema, - expectedText1: 'This feature flag is False in development because', + expectedText1: /This feature flag is False in development because/, expectedText2: 'all strategies are either False or could not be fully evaluated', }, ]; +testCases.forEach(({ name, feature, expectedText1, expectedText2 }) => { + test(`${name} (legacy)`, async () => { + render( + {}} + />, + ); + + await screen.findByText(expectedText1); + await screen.findByText(expectedText2); + }); +}); + testCases.forEach(({ name, feature, expectedText1, expectedText2 }) => { test(name, async () => { render( diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureDetails/FeatureDetails.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureDetails/FeatureDetails.tsx index 4ebf9e60cc..e56eefb55f 100644 --- a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureDetails/FeatureDetails.tsx +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureDetails/FeatureDetails.tsx @@ -1,45 +1,32 @@ import type { PlaygroundFeatureSchema, PlaygroundRequestSchema } from 'openapi'; -import { Alert, IconButton, Typography, useTheme, styled } from '@mui/material'; +import { Alert, Typography, useTheme, styled, IconButton } from '@mui/material'; import { PlaygroundResultChip } from '../../PlaygroundResultChip/PlaygroundResultChip'; import CloseOutlined from '@mui/icons-material/CloseOutlined'; import type React from 'react'; -import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import { checkForEmptyValues, hasCustomStrategies, hasOnlyCustomStrategies, } from './helpers'; -const StyledDivWrapper = styled('div')({ +const HeaderRow = styled('div')({ display: 'flex', justifyContent: 'space-between', width: '100%', }); -const StyledDivTitleRow = styled('div')(({ theme }) => ({ +const HeaderGroup = styled('hgroup')(({ theme }) => ({ display: 'inline-flex', alignItems: 'center', gap: theme.spacing(1.5), - marginTop: theme.spacing(1.5), })); -const StyledDivAlertRow = styled('div')(({ theme }) => ({ - margin: theme.spacing(1, 0), +const StyledTypographyName = styled('h3')(({ theme }) => ({ + fontWeight: 'bold', + fontSize: theme.typography.subtitle1.fontSize, + margin: 0, })); -const StyledDivDescriptionRow = styled('div')(({ theme }) => ({ - margin: theme.spacing(1, 0.5), -})); - -const StyledTypographyName = styled(Typography)(({ theme }) => ({ - fontWeight: 600, - padding: theme.spacing(0.5), -})); - -const StyledIconButton = styled(IconButton)({ - textAlign: 'right', -}); - interface PlaygroundFeatureResultDetailsProps { feature: PlaygroundFeatureSchema; input?: PlaygroundRequestSchema; @@ -57,7 +44,7 @@ export const FeatureDetails = ({ return [ `This feature flag is True in ${input?.environment} because `, 'at least one strategy is True', - theme.palette.success.main, + theme.palette.success.contrastText, ]; if ( @@ -67,7 +54,7 @@ export const FeatureDetails = ({ return [ `This feature flag is False in ${input?.environment} because `, 'parent dependency is not satisfied and the environment is disabled', - theme.palette.error.main, + theme.palette.error.contrastText, ]; } @@ -75,35 +62,35 @@ export const FeatureDetails = ({ return [ `This feature flag is False in ${input?.environment} because `, 'the environment is disabled', - theme.palette.error.main, + theme.palette.error.contrastText, ]; if (hasOnlyCustomStrategies(feature)) return [ `This feature flag is Unknown in ${input?.environment} because `, 'no strategies could be fully evaluated', - theme.palette.warning.main, + theme.palette.warning.contrastText, ]; if (hasCustomStrategies(feature)) return [ `This feature flag is Unknown in ${input?.environment} because `, 'not all strategies could be fully evaluated', - theme.palette.warning.main, + theme.palette.warning.contrastText, ]; if (feature.hasUnsatisfiedDependency) { return [ `This feature flag is False in ${input?.environment} because `, 'parent dependency is not satisfied', - theme.palette.error.main, + theme.palette.error.contrastText, ]; } return [ `This feature flag is False in ${input?.environment} because `, 'all strategies are either False or could not be fully evaluated', - theme.palette.error.main, + theme.palette.error.contrastText, ]; })(); @@ -124,61 +111,43 @@ export const FeatureDetails = ({ return ( <> - - - - {feature.name} - - ( + + + {feature.name} +

+ {feature?.strategies?.result !== 'unknown' ? ( - )} - elseShow={() => ( + ) : ( )} - /> - - +

+ + - - - - - {description} - - + + +

+ {description} + {reason} - - . - - - - {noValueTxt} - - } - /> - - - {customStrategiesTxt} - - - } - /> + . +

+ {noValueTxt ? {noValueTxt} : null} + {customStrategiesTxt ? ( + + {customStrategiesTxt} + + ) : null} ); }; diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureDetails/LegacyFeatureDetails.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureDetails/LegacyFeatureDetails.tsx new file mode 100644 index 0000000000..4ebf9e60cc --- /dev/null +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureDetails/LegacyFeatureDetails.tsx @@ -0,0 +1,184 @@ +import type { PlaygroundFeatureSchema, PlaygroundRequestSchema } from 'openapi'; +import { Alert, IconButton, Typography, useTheme, styled } from '@mui/material'; +import { PlaygroundResultChip } from '../../PlaygroundResultChip/PlaygroundResultChip'; +import CloseOutlined from '@mui/icons-material/CloseOutlined'; +import type React from 'react'; +import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; +import { + checkForEmptyValues, + hasCustomStrategies, + hasOnlyCustomStrategies, +} from './helpers'; + +const StyledDivWrapper = styled('div')({ + display: 'flex', + justifyContent: 'space-between', + width: '100%', +}); + +const StyledDivTitleRow = styled('div')(({ theme }) => ({ + display: 'inline-flex', + alignItems: 'center', + gap: theme.spacing(1.5), + marginTop: theme.spacing(1.5), +})); + +const StyledDivAlertRow = styled('div')(({ theme }) => ({ + margin: theme.spacing(1, 0), +})); + +const StyledDivDescriptionRow = styled('div')(({ theme }) => ({ + margin: theme.spacing(1, 0.5), +})); + +const StyledTypographyName = styled(Typography)(({ theme }) => ({ + fontWeight: 600, + padding: theme.spacing(0.5), +})); + +const StyledIconButton = styled(IconButton)({ + textAlign: 'right', +}); + +interface PlaygroundFeatureResultDetailsProps { + feature: PlaygroundFeatureSchema; + input?: PlaygroundRequestSchema; + onClose: () => void; +} +export const FeatureDetails = ({ + feature, + input, + onClose, +}: PlaygroundFeatureResultDetailsProps) => { + const theme = useTheme(); + + const [description, reason, color] = (() => { + if (feature.isEnabled) + return [ + `This feature flag is True in ${input?.environment} because `, + 'at least one strategy is True', + theme.palette.success.main, + ]; + + if ( + feature.hasUnsatisfiedDependency && + !feature.isEnabledInCurrentEnvironment + ) { + return [ + `This feature flag is False in ${input?.environment} because `, + 'parent dependency is not satisfied and the environment is disabled', + theme.palette.error.main, + ]; + } + + if (!feature.isEnabledInCurrentEnvironment) + return [ + `This feature flag is False in ${input?.environment} because `, + 'the environment is disabled', + theme.palette.error.main, + ]; + + if (hasOnlyCustomStrategies(feature)) + return [ + `This feature flag is Unknown in ${input?.environment} because `, + 'no strategies could be fully evaluated', + theme.palette.warning.main, + ]; + + if (hasCustomStrategies(feature)) + return [ + `This feature flag is Unknown in ${input?.environment} because `, + 'not all strategies could be fully evaluated', + theme.palette.warning.main, + ]; + + if (feature.hasUnsatisfiedDependency) { + return [ + `This feature flag is False in ${input?.environment} because `, + 'parent dependency is not satisfied', + theme.palette.error.main, + ]; + } + + return [ + `This feature flag is False in ${input?.environment} because `, + 'all strategies are either False or could not be fully evaluated', + theme.palette.error.main, + ]; + })(); + + const noValueTxt = checkForEmptyValues(input?.context) + ? 'You did not provide a value for your context field in step 2 of the configuration' + : undefined; + + const customStrategiesTxt = hasCustomStrategies(feature) + ? `This feature uses custom strategies. Custom strategies can't be evaluated, so they will be marked accordingly.` + : undefined; + + const onCloseClick = + onClose && + ((event: React.SyntheticEvent) => { + event.stopPropagation(); + onClose(); + }); + + return ( + <> + + + + {feature.name} + + ( + + )} + elseShow={() => ( + + )} + /> + + + + + + + + {description} + + + {reason} + + + . + + + + {noValueTxt} + + } + /> + + + {customStrategiesTxt} + + + } + /> + + ); +}; diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureResultInfoPopoverCell.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureResultInfoPopoverCell.tsx index 7cb05e44f2..dc5b418e9f 100644 --- a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureResultInfoPopoverCell.tsx +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureResultInfoPopoverCell.tsx @@ -2,8 +2,11 @@ import { useRef, useState } from 'react'; import type { PlaygroundFeatureSchema, PlaygroundRequestSchema } from 'openapi'; import { IconButton, Popover, styled } from '@mui/material'; import InfoOutlined from '@mui/icons-material/InfoOutlined'; +import { FeatureDetails as LegacyFeatureDetails } from './FeatureDetails/LegacyFeatureDetails'; +import { PlaygroundResultFeatureStrategyList as LegacyPlaygroundResultFeatureStrategyList } from './FeatureStrategyList/LegacyPlaygroundResultFeatureStrategyList'; +import { useUiFlag } from 'hooks/useUiFlag'; import { FeatureDetails } from './FeatureDetails/FeatureDetails'; -import { PlaygroundResultFeatureStrategyList } from './FeatureStrategyList/PlaygroundResultFeatureStrategyList'; +import { PlaygroundResultFeatureStrategyList } from './FeatureStrategyList/PlaygroundResultsFeatureStrategyList'; interface FeatureResultInfoPopoverCellProps { feature: PlaygroundFeatureSchema; @@ -21,6 +24,7 @@ export const FeatureResultInfoPopoverCell = ({ }: FeatureResultInfoPopoverCellProps) => { const [open, setOpen] = useState(false); const ref = useRef(null); + const useNewStrategyDesign = useUiFlag('flagOverviewRedesign'); const togglePopover = () => { setOpen(!open); @@ -43,7 +47,7 @@ export const FeatureResultInfoPopoverCell = ({ sx: (theme) => ({ display: 'flex', flexDirection: 'column', - padding: theme.spacing(6), + padding: theme.spacing(useNewStrategyDesign ? 4 : 6), width: 728, maxWidth: '100%', height: 'auto', @@ -61,15 +65,31 @@ export const FeatureResultInfoPopoverCell = ({ horizontal: 'left', }} > - setOpen(false)} - /> - + {useNewStrategyDesign ? ( + <> + setOpen(false)} + /> + + + ) : ( + <> + setOpen(false)} + /> + + + )} ); diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultFeatureStrategyList.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/LegacyPlaygroundResultFeatureStrategyList.tsx similarity index 100% rename from frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultFeatureStrategyList.tsx rename to frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/LegacyPlaygroundResultFeatureStrategyList.tsx diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultFeatureStrategyList.test.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultFeatureStrategyList.test.tsx index 24340f967c..b64be1ee17 100644 --- a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultFeatureStrategyList.test.tsx +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultFeatureStrategyList.test.tsx @@ -1,8 +1,9 @@ import { screen } from '@testing-library/react'; import { render } from 'utils/testRenderer'; import type { PlaygroundFeatureSchema, PlaygroundRequestSchema } from 'openapi'; -import { PlaygroundResultFeatureStrategyList } from './PlaygroundResultFeatureStrategyList'; +import { PlaygroundResultFeatureStrategyList as LegacyPlaygroundResultFeatureStrategyList } from './LegacyPlaygroundResultFeatureStrategyList'; import { vi } from 'vitest'; +import { PlaygroundResultFeatureStrategyList } from './PlaygroundResultsFeatureStrategyList'; const testCases = [ { @@ -135,6 +136,21 @@ afterAll(() => { vi.clearAllMocks(); }); +testCases.forEach(({ name, feature, expectedText }) => { + test(`${name} (legacy)`, async () => { + render( + , + ); + + await screen.findByText(expectedText); + }); +}); + testCases.forEach(({ name, feature, expectedText }) => { test(name, async () => { render( diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultsFeatureStrategyList.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultsFeatureStrategyList.tsx new file mode 100644 index 0000000000..2a3e1b25b3 --- /dev/null +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/PlaygroundResultsFeatureStrategyList.tsx @@ -0,0 +1,67 @@ +import { + PlaygroundResultStrategyLists, + WrappedPlaygroundResultStrategyList, +} from './StrategyList/playgroundResultStrategyLists'; +import type { PlaygroundFeatureSchema, PlaygroundRequestSchema } from 'openapi'; +import { Alert } from '@mui/material'; + +interface PlaygroundResultFeatureStrategyListProps { + feature: PlaygroundFeatureSchema; + input?: PlaygroundRequestSchema; +} + +export const PlaygroundResultFeatureStrategyList = ({ + feature, + input, +}: PlaygroundResultFeatureStrategyListProps) => { + const enabledStrategies = feature.strategies?.data?.filter( + (strategy) => !strategy.disabled, + ); + const disabledStrategies = feature.strategies?.data?.filter( + (strategy) => strategy.disabled, + ); + + const showDisabledStrategies = disabledStrategies?.length > 0; + + if ((feature?.strategies?.data.length ?? 0) === 0) { + return ( + + There are no strategies added to this feature flag in the + selected environment. + + ); + } + + if ( + (feature.hasUnsatisfiedDependency || + !feature.isEnabledInCurrentEnvironment) && + Boolean(feature?.strategies?.data) + ) { + return ( + + ); + } + + return ( + <> + + {showDisabledStrategies ? ( + + ) : null} + + ); +}; diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/PlaygroundResultChip/PlaygroundResultChip.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/PlaygroundResultChip/PlaygroundResultChip.tsx index c5bd63cce7..f7ef9883c9 100644 --- a/frontend/src/component/playground/Playground/PlaygroundResultsTable/PlaygroundResultChip/PlaygroundResultChip.tsx +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/PlaygroundResultChip/PlaygroundResultChip.tsx @@ -11,12 +11,14 @@ interface IResultChipProps { label: string; // Result icon - defaults to true showIcon?: boolean; + tabindex?: number; } export const PlaygroundResultChip: VFC = ({ enabled, label, showIcon = true, + tabindex, }) => { const theme = useTheme(); const icon = ( @@ -28,12 +30,14 @@ export const PlaygroundResultChip: VFC = ({ condition={typeof enabled === 'boolean' && Boolean(enabled)} show={ } elseShow={ @@ -56,6 +60,7 @@ export const PlaygroundResultChip: VFC = ({ condition={typeof enabled === 'boolean' && Boolean(enabled)} show={ @@ -63,7 +68,11 @@ export const PlaygroundResultChip: VFC = ({ } elseShow={ - + {label} }