From c868b5a868ce15b61f5d7ebe98dfdebd6f00727b Mon Sep 17 00:00:00 2001 From: andreas-unleash Date: Fri, 29 Mar 2024 15:44:34 +0200 Subject: [PATCH] Feat: context field search and filter improvements (#6732) Adds highlighting to search values Search also looks in `description` behind a flag - it could possibly degrade performance when too many items. Tested with 200 and it's ok but anything above might degrade: Adds a Select/Unselect all button Shows the selected values above the search Closes # [1-2232](https://linear.app/unleash/issue/1-2232/context-field-ui-filter-and-search) https://github.com/Unleash/unleash/assets/104830839/ba2fe56f-c5db-4ce7-bc3c-1e7988682984 --------- Signed-off-by: andreas-unleash --- .../ConstraintValueSearch.tsx | 15 +--- .../ConstraintFormHeader.tsx | 2 - .../LegalValueLabel/LegalValueLabel.styles.ts | 2 +- .../LegalValueLabel/LegalValueLabel.tsx | 23 +++++- .../filter-legal-values.test.ts | 75 ++++++++++++++++++ .../RestrictiveLegalValues.test.tsx | 69 +++++++++++++++- .../RestrictiveLegalValues.tsx | 78 ++++++++++++++++--- frontend/src/interfaces/uiConfig.ts | 1 + .../__snapshots__/create-config.test.ts.snap | 1 + src/lib/types/experimental.ts | 7 +- src/server-dev.ts | 1 + 11 files changed, 240 insertions(+), 34 deletions(-) create mode 100644 frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/LegalValueLabel/filter-legal-values.test.ts diff --git a/frontend/src/component/common/ConstraintAccordion/ConstraintValueSearch/ConstraintValueSearch.tsx b/frontend/src/component/common/ConstraintAccordion/ConstraintValueSearch/ConstraintValueSearch.tsx index 47d688e493..b7337519eb 100644 --- a/frontend/src/component/common/ConstraintAccordion/ConstraintValueSearch/ConstraintValueSearch.tsx +++ b/frontend/src/component/common/ConstraintAccordion/ConstraintValueSearch/ConstraintValueSearch.tsx @@ -1,6 +1,5 @@ -import { TextField, InputAdornment, Chip } from '@mui/material'; +import { TextField, InputAdornment } from '@mui/material'; import Search from '@mui/icons-material/Search'; -import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; interface IConstraintValueSearchProps { filter: string; @@ -13,7 +12,7 @@ export const ConstraintValueSearch = ({ }: IConstraintValueSearchProps) => { return (
-
+
- setFilter('')} - /> - } - />
); }; diff --git a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ConstraintFormHeader/ConstraintFormHeader.tsx b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ConstraintFormHeader/ConstraintFormHeader.tsx index 90b0301d6a..bce4c4ec16 100644 --- a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ConstraintFormHeader/ConstraintFormHeader.tsx +++ b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ConstraintFormHeader/ConstraintFormHeader.tsx @@ -4,8 +4,6 @@ import { styled } from '@mui/material'; const StyledHeader = styled('h3')(({ theme }) => ({ fontSize: theme.fontSizes.bodySize, fontWeight: theme.typography.fontWeightRegular, - marginTop: theme.spacing(2), - marginBottom: theme.spacing(0.5), })); export const ConstraintFormHeader: React.FC< diff --git a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/LegalValueLabel/LegalValueLabel.styles.ts b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/LegalValueLabel/LegalValueLabel.styles.ts index 41a457ead5..f4d58d2c0f 100644 --- a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/LegalValueLabel/LegalValueLabel.styles.ts +++ b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/LegalValueLabel/LegalValueLabel.styles.ts @@ -9,7 +9,7 @@ export const StyledContainer = styled('div')(({ theme }) => ({ borderRadius: theme.shape.borderRadius, '&:hover': { - border: `2px solid ${theme.palette.primary.main}`, + border: `1px solid ${theme.palette.primary.main}`, }, })); diff --git a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/LegalValueLabel/LegalValueLabel.tsx b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/LegalValueLabel/LegalValueLabel.tsx index b02bdca279..142389eb3d 100644 --- a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/LegalValueLabel/LegalValueLabel.tsx +++ b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/LegalValueLabel/LegalValueLabel.tsx @@ -6,13 +6,19 @@ import { } from './LegalValueLabel.styles'; import type React from 'react'; import { FormControlLabel } from '@mui/material'; +import { Highlighter } from 'component/common/Highlighter/Highlighter'; interface ILegalValueTextProps { legal: ILegalValue; control: React.ReactElement; + filter?: string; } -export const LegalValueLabel = ({ legal, control }: ILegalValueTextProps) => { +export const LegalValueLabel = ({ + legal, + control, + filter, +}: ILegalValueTextProps) => { return ( { control={control} label={ <> - {legal.value} + + + {legal.value} + + - {legal.description} + + {legal.description} + } @@ -36,6 +48,9 @@ export const filterLegalValues = ( filter: string, ): ILegalValue[] => { return legalValues.filter((legalValue) => { - return legalValue.value.includes(filter); + return ( + legalValue.value.toLowerCase().includes(filter.toLowerCase()) || + legalValue.description?.toLowerCase().includes(filter.toLowerCase()) + ); }); }; diff --git a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/LegalValueLabel/filter-legal-values.test.ts b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/LegalValueLabel/filter-legal-values.test.ts new file mode 100644 index 0000000000..cf0ab3e5fd --- /dev/null +++ b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/LegalValueLabel/filter-legal-values.test.ts @@ -0,0 +1,75 @@ +import { filterLegalValues } from './LegalValueLabel'; + +describe('filterLegalValues function tests', () => { + const mockLegalValues = [ + { value: 'Apple', description: 'A fruit' }, + { value: 'Banana', description: 'Yellow fruit' }, + { value: 'Carrot', description: 'A vegetable' }, + { value: 'SE', description: 'Sweden' }, + { value: 'Eggplant', description: undefined }, + ]; + + test('Basic functionality with value property', () => { + const filter = 'apple'; + const expected = [{ value: 'Apple', description: 'A fruit' }]; + expect(filterLegalValues(mockLegalValues, filter)).toEqual(expected); + }); + + test('Filters based on description property', () => { + const filter = 'vegetable'; + const expected = [{ value: 'Carrot', description: 'A vegetable' }]; + expect(filterLegalValues(mockLegalValues, filter)).toEqual(expected); + }); + + test('Case insensitivity', () => { + const filter = 'BANANA'; + const expected = [{ value: 'Banana', description: 'Yellow fruit' }]; + expect(filterLegalValues(mockLegalValues, filter)).toEqual(expected); + }); + + test('No matches found', () => { + const filter = 'Zucchini'; + expect(filterLegalValues(mockLegalValues, filter)).toEqual([]); + }); + + test('Empty filter string', () => { + const filter = ''; + expect(filterLegalValues(mockLegalValues, filter)).toEqual( + mockLegalValues, + ); + }); + + test('Special characters in filter', () => { + const filter = 'a fruit'; + const expected = [{ value: 'Apple', description: 'A fruit' }]; + expect(filterLegalValues(mockLegalValues, filter)).toEqual(expected); + }); + + test('Empty input array', () => { + const filter = 'anything'; + expect(filterLegalValues([], filter)).toEqual([]); + }); + + test('Exact match', () => { + const filter = 'Carrot'; + const expected = [{ value: 'Carrot', description: 'A vegetable' }]; + expect(filterLegalValues(mockLegalValues, filter)).toEqual(expected); + }); + + test('Partial match', () => { + const filter = 'sw'; + const expected = [{ value: 'SE', description: 'Sweden' }]; + expect(filterLegalValues(mockLegalValues, filter)).toEqual(expected); + }); + + test('Combination of match and no match', () => { + const filter = 'a'; + const expected = [ + { value: 'Apple', description: 'A fruit' }, + { value: 'Banana', description: 'Yellow fruit' }, + { value: 'Carrot', description: 'A vegetable' }, + { value: 'Eggplant', description: undefined }, + ]; + expect(filterLegalValues(mockLegalValues, filter)).toEqual(expected); + }); +}); diff --git a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.test.tsx b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.test.tsx index 6039281dc8..0f68f81ced 100644 --- a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.test.tsx +++ b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.test.tsx @@ -1,6 +1,11 @@ import { render } from 'utils/testRenderer'; -import { screen } from '@testing-library/react'; +import { fireEvent, screen } from '@testing-library/react'; import { RestrictiveLegalValues } from './RestrictiveLegalValues'; +import { vi } from 'vitest'; + +vi.mock('../../../../../../hooks/useUiFlag', () => ({ + useUiFlag: vi.fn(() => true), +})); test('should show alert when you have illegal legal values', async () => { const contextDefinitionValues = [{ value: 'value1' }, { value: 'value2' }]; @@ -52,3 +57,65 @@ test('Should remove illegal legal values from internal value state when mounting expect(localValues).toEqual(['value2']); }); + +test('Should select all', async () => { + const contextDefinitionValues = [{ value: 'value1' }, { value: 'value2' }]; + let localValues: string[] = []; + + const setValuesWithRecord = (values: string[]) => { + localValues = values; + }; + + render( + {}} + setValuesWithRecord={setValuesWithRecord} + error={''} + setError={() => {}} + />, + ); + + const selectedAllButton = await screen.findByText(/Select all/i); + + console.log(selectedAllButton); + + fireEvent.click(selectedAllButton); + expect(localValues).toEqual(['value1', 'value2']); +}); + +test('Should unselect all', async () => { + const contextDefinitionValues = [{ value: 'value1' }, { value: 'value2' }]; + let localValues: string[] = ['value1', 'value2']; + + const setValuesWithRecord = (values: string[]) => { + localValues = values; + }; + + render( + {}} + setValuesWithRecord={setValuesWithRecord} + error={''} + setError={() => {}} + />, + ); + + const selectedAllButton = await screen.findByText(/Unselect all/i); + + console.log(selectedAllButton); + + fireEvent.click(selectedAllButton); + expect(localValues).toEqual([]); +}); diff --git a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.tsx b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.tsx index 0751febb77..864d70bf26 100644 --- a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.tsx +++ b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/RestrictiveLegalValues/RestrictiveLegalValues.tsx @@ -1,7 +1,6 @@ import { useEffect, useState } from 'react'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; -import { Alert, Checkbox, styled } from '@mui/material'; -import { useThemeStyles } from 'themes/themeStyles'; +import { Alert, Button, Checkbox, Chip, Stack, styled } from '@mui/material'; import { ConstraintValueSearch } from 'component/common/ConstraintAccordion/ConstraintValueSearch/ConstraintValueSearch'; import { ConstraintFormHeader } from '../ConstraintFormHeader/ConstraintFormHeader'; import type { ILegalValue } from 'interfaces/context'; @@ -9,6 +8,7 @@ import { filterLegalValues, LegalValueLabel, } from '../LegalValueLabel/LegalValueLabel'; +import { useUiFlag } from 'hooks/useUiFlag'; interface IRestrictiveLegalValuesProps { data: { @@ -60,6 +60,16 @@ const StyledValuesContainer = styled('div')(({ theme }) => ({ maxHeight: '378px', overflow: 'auto', })); +const StyledStack = styled(Stack)(({ theme }) => ({ + marginTop: theme.spacing(2), + marginBottom: theme.spacing(0.5), + justifyContent: 'space-between', +})); + +const ErrorText = styled('p')(({ theme }) => ({ + fontSize: theme.fontSizes.smallBody, + color: theme.palette.error.main, +})); export const RestrictiveLegalValues = ({ data, @@ -77,7 +87,8 @@ export const RestrictiveLegalValues = ({ // Lazily initialise the values because there might be a lot of them. const [valuesMap, setValuesMap] = useState(() => createValuesMap(values)); - const { classes: styles } = useThemeStyles(); + + const newContextFieldsUI = useUiFlag('newContextFieldsUI'); const cleanDeletedLegalValues = (constraintValues: string[]): string[] => { const deletedValuesSet = getLegalValueSet(deletedLegalValues); @@ -116,6 +127,19 @@ export const RestrictiveLegalValues = ({ setValuesWithRecord([...cleanDeletedLegalValues(values), legalValue]); }; + const isAllSelected = legalValues.every((value) => + values.includes(value.value), + ); + + const onSelectAll = () => { + if (isAllSelected) { + return setValuesWithRecord([]); + } + setValuesWithRecord([ + ...legalValues.map((legalValue) => legalValue.value), + ]); + }; + return ( <> } /> - - - Select values from a predefined set - + + + Select values from a predefined set + + + {isAllSelected ? 'Unselect all' : 'Select all'} + + } + /> + 100} show={ - + <> + + {values.map((value) => { + return ( + onChange(value)} + /> + ); + })} + + } + /> + + } /> @@ -152,6 +205,7 @@ export const RestrictiveLegalValues = ({ {error}

} + show={{error}} /> ); diff --git a/frontend/src/interfaces/uiConfig.ts b/frontend/src/interfaces/uiConfig.ts index dd6bb98e82..4903f50a06 100644 --- a/frontend/src/interfaces/uiConfig.ts +++ b/frontend/src/interfaces/uiConfig.ts @@ -78,6 +78,7 @@ export type UiFlags = { outdatedSdksBanner?: boolean; projectOverviewRefactor?: string; collectTrafficDataUsage?: boolean; + newContextFieldsUI?: boolean; variantDependencies?: boolean; }; diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index 4e0c02fd68..bbf208ffdf 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/src/lib/__snapshots__/create-config.test.ts.snap @@ -128,6 +128,7 @@ exports[`should create default config 1`] = ` }, }, "migrationLock": true, + "newContextFieldsUI": false, "newStrategyConfigurationFeedback": false, "outdatedSdksBanner": false, "personalAccessTokensKillSwitch": false, diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index 7a73c1024b..325e42d416 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -55,7 +55,8 @@ export type IFlagKey = | 'globalFrontendApiCache' | 'returnGlobalFrontendApiCache' | 'projectOverviewRefactor' - | 'variantDependencies'; + | 'variantDependencies' + | 'newContextFieldsUI'; export type IFlags = Partial<{ [key in IFlagKey]: boolean | Variant }>; @@ -268,6 +269,10 @@ const flags: IFlags = { process.env.UNLEASH_EXPERIMENTAL_PROJECT_OVERVIEW_REFACTOR, false, ), + newContextFieldsUI: parseEnvVarBoolean( + process.env.UNLEASH_EXPERIMENTAL_NEW_CONTEXT_FIELDS_UI, + false, + ), variantDependencies: parseEnvVarBoolean( process.env.UNLEASH_EXPERIMENTAL_VARIANT_DEPENDENCIES, false, diff --git a/src/server-dev.ts b/src/server-dev.ts index 669f01f4a2..f8ed65f9ff 100644 --- a/src/server-dev.ts +++ b/src/server-dev.ts @@ -52,6 +52,7 @@ process.nextTick(async () => { globalFrontendApiCache: true, returnGlobalFrontendApiCache: false, projectOverviewRefactor: true, + newContextFieldsUI: true, variantDependencies: true, }, },