diff --git a/frontend/documentation/components/InlinePillToggle.stories.tsx b/frontend/documentation/components/InlinePillToggle.stories.tsx new file mode 100644 index 000000000000..560a230596b7 --- /dev/null +++ b/frontend/documentation/components/InlinePillToggle.stories.tsx @@ -0,0 +1,98 @@ +import React, { useState } from 'react' +import type { Meta, StoryObj } from 'storybook' + +import InlinePillToggle from 'components/base/forms/InlinePillToggle' + +const meta: Meta = { + parameters: { + docs: { + description: { + component: + 'A compact inline segmented control for toggling between two or more options. Available in small, medium, and large sizes.', + }, + }, + layout: 'padded', + }, + title: 'Components/Forms/InlinePillToggle', +} +export default meta + +type Story = StoryObj + +const options = [ + { label: 'ALL', value: 'ALL' }, + { label: 'ANY', value: 'ANY' }, +] + +function SmallExample() { + const [value, setValue] = useState('ALL') + return ( + + ) +} + +function MediumExample() { + const [value, setValue] = useState('ALL') + return ( + + ) +} + +function LargeExample() { + const [value, setValue] = useState('ALL') + return ( + + ) +} + +function InlineWithTextExample() { + const [value, setValue] = useState('ALL') + return ( + + Include users when{' '} + {' '} + of the following rules apply: + + ) +} + +function ThreeOptionsExample() { + const [value, setValue] = useState('day') + return ( + + ) +} + +export const Small: Story = { render: () => } +export const Medium: Story = { render: () => } +export const Large: Story = { render: () => } +export const InlineWithText: Story = { render: () => } +export const ThreeOptions: Story = { render: () => } diff --git a/frontend/e2e/helpers/e2e-helpers.playwright.ts b/frontend/e2e/helpers/e2e-helpers.playwright.ts index ef09172c9399..543efe790b67 100644 --- a/frontend/e2e/helpers/e2e-helpers.playwright.ts +++ b/frontend/e2e/helpers/e2e-helpers.playwright.ts @@ -511,9 +511,17 @@ export class E2EHelpers { } // Create a segment - async createSegment(name: string, rules?: Rule[]) { + async createSegment( + name: string, + rules?: Rule[], + topLevelRuleType: 'ALL' | 'ANY' = 'ALL', + ) { await this.click(byId('show-create-segment-btn')); await this.setText(byId('segmentID'), name); + const flagsmith = await getFlagsmith(); + if (flagsmith.hasFeature('segment_any_rule_type')) { + await this.click(byId(`top-level-rule-type-${topLevelRuleType}`)); + } if (rules && rules.length > 0) { for (let x = 0; x < rules.length; x++) { const rule = rules[x]; diff --git a/frontend/e2e/tests/segment-test.pw.ts b/frontend/e2e/tests/segment-test.pw.ts index 3f0455741055..e255a7aef2e8 100644 --- a/frontend/e2e/tests/segment-test.pw.ts +++ b/frontend/e2e/tests/segment-test.pw.ts @@ -1,5 +1,5 @@ import { test, expect } from '../test-setup'; -import { byId, log, createHelpers, visualSnapshot } from '../helpers'; +import { byId, log, createHelpers, visualSnapshot, getFlagsmith } from '../helpers'; import { E2E_USER, PASSWORD, E2E_TEST_IDENTITY, E2E_SEGMENT_PROJECT_1, E2E_SEGMENT_PROJECT_2, E2E_SEGMENT_PROJECT_3 } from '../config' const REMOTE_CONFIG_FEATURE = 'remote_config' @@ -53,6 +53,35 @@ const segmentRules = [ }, ] +// (age_any = 18 AND team = "alpha") OR (age_any = 25 AND team = "beta") +// Note: `ors` field is reused for "additional conditions in the same group" — in ANY mode these are AND-ed. +const segmentAnyRules = [ + { + name: 'age_any', + operator: 'EQUAL', + value: 18, + ors: [ + { + name: 'team', + operator: 'EQUAL', + value: 'alpha', + }, + ], + }, + { + name: 'age_any', + operator: 'EQUAL', + value: 25, + ors: [ + { + name: 'team', + operator: 'EQUAL', + value: 'beta', + }, + ], + }, +] + test('Segment test 1 - Create, update, and manage segments with multivariate flags @oss', async ({ page }, testInfo) => { const { addSegmentOverride, @@ -359,3 +388,89 @@ test('Segment test 3 - Test user-specific feature overrides @oss', async ({ page await deleteFeature(FLAG_FEATURE) await deleteFeature(REMOTE_CONFIG_FEATURE) }) + +test('Segment test 4 - Create ANY rule type segment and verify match changes when rule is updated @oss', async ({ page }) => { + const ANY_FEATURE = 'any_segment_feature' + const ANY_SEGMENT = 'any_segment_test' + const { + addSegmentOverrideConfig, + assertUserFeatureValue, + click, + createRemoteConfig, + createSegment, + createTrait, + deleteFeature, + deleteSegment, + deleteTrait, + goToUser, + gotoFeature, + gotoFeatures, + gotoProject, + gotoSegments, + gotoTraits, + login, + navigateToSegment, + saveFeatureSegments, + setSegmentRule, + waitAndRefresh, + waitForElementVisible, + } = createHelpers(page) + const flagsmith = await getFlagsmith() + const hasFeature = flagsmith.hasFeature('segment_any_rule_type') + + log('Login') + await login(E2E_USER, PASSWORD) + + if (!hasFeature) { + log('Skipping ANY segment test, feature not enabled.') + test.skip() + return + } + + await gotoProject(E2E_SEGMENT_PROJECT_1) + await waitForElementVisible(byId('features-page')) + + log('Create remote config feature with default value') + await createRemoteConfig({ name: ANY_FEATURE, value: 'default' }) + + log('Set traits matching the first ANY group') + await gotoTraits(E2E_TEST_IDENTITY) + await createTrait('age_any', 18) + await createTrait('team', 'alpha') + + log('Create ANY-mode segment') + await gotoSegments() + await createSegment(ANY_SEGMENT, segmentAnyRules, 'ANY') + + log('Override feature value via segment') + await gotoFeatures() + await gotoFeature(ANY_FEATURE) + await addSegmentOverrideConfig(0, 'overridden', 0) + await saveFeatureSegments() + + log('Verify user is in the segment (gets overridden value)') + await goToUser(E2E_TEST_IDENTITY) + await waitAndRefresh() + await assertUserFeatureValue(ANY_FEATURE, '"overridden"') + + log('Update segment so user no longer matches') + await gotoSegments() + await navigateToSegment(ANY_SEGMENT) + // Change the first group's `team` condition from "alpha" to "gamma" — user has team=alpha so no longer matches. + await setSegmentRule(0, 1, 'team', 'EQUAL', 'gamma') + await click(byId('update-segment')) + + log('Verify user is no longer in the segment (gets default value)') + await goToUser(E2E_TEST_IDENTITY) + await waitAndRefresh() + await assertUserFeatureValue(ANY_FEATURE, '"default"') + + log('Clean up feature, segment, and traits') + await gotoFeatures() + await deleteFeature(ANY_FEATURE) + await gotoSegments() + await deleteSegment(ANY_SEGMENT) + await gotoTraits(E2E_TEST_IDENTITY) + await deleteTrait('age_any') + await deleteTrait('team') +}) diff --git a/frontend/web/components/SegmentRuleDivider.tsx b/frontend/web/components/SegmentRuleDivider.tsx index 5d64c8e8c5d3..a1958842b919 100644 --- a/frontend/web/components/SegmentRuleDivider.tsx +++ b/frontend/web/components/SegmentRuleDivider.tsx @@ -7,30 +7,36 @@ type SegmentRuleDividerType = { rule: SegmentRule index: number className?: string + topLevelRuleType?: 'ALL' | 'ANY' +} + +const typeLabel: Record = { + ALL: 'All of the following', + ANY: 'Any of the following', + NONE: 'None of the following', } const SegmentRuleDivider: FC = ({ className, index, rule, + topLevelRuleType = 'ALL', }) => { - if (rule?.type === 'ALL') return null + if (rule?.type === 'ALL' && topLevelRuleType === 'ALL') return null + const connector = topLevelRuleType === 'ANY' ? 'Or' : 'And' + const prefix = index > 0 ? `${connector} ` : '' return ( - {Format.camelCase( - `${index > 0 ? 'And ' : ''}${ - rule.type === 'ANY' ? 'Any of the following' : 'None of the following' - }`, - )} + {Format.camelCase(`${prefix}${typeLabel[rule.type]}`)} ) diff --git a/frontend/web/components/base/forms/InlinePillToggle.tsx b/frontend/web/components/base/forms/InlinePillToggle.tsx new file mode 100644 index 000000000000..65ba8053fffc --- /dev/null +++ b/frontend/web/components/base/forms/InlinePillToggle.tsx @@ -0,0 +1,81 @@ +import React, { useRef } from 'react' +import cn from 'classnames' + +type InlinePillToggleSize = 'small' | 'medium' | 'large' + +type Option = { + label: string + value: T +} + +type InlinePillToggleProps = { + options: Option[] + value: T + onChange: (value: T) => void + size?: InlinePillToggleSize + className?: string + 'data-test'?: string +} + +function InlinePillToggle({ + className, + 'data-test': dataTest, + onChange, + options, + size = 'medium', + value, +}: InlinePillToggleProps) { + const buttonRefs = useRef<(HTMLButtonElement | null)[]>([]) + + const handleKeyDown = (e: React.KeyboardEvent, index: number) => { + let next: number | null = null + if (e.key === 'ArrowRight' || e.key === 'ArrowDown') { + next = (index + 1) % options.length + } else if (e.key === 'ArrowLeft' || e.key === 'ArrowUp') { + next = (index - 1 + options.length) % options.length + } + if (next !== null) { + e.preventDefault() + onChange(options[next].value) + buttonRefs.current[next]?.focus() + } + } + + return ( +
+ {options.map((option, index) => { + const isActive = value === option.value + return ( + + ) + })} +
+ ) +} + +export default InlinePillToggle diff --git a/frontend/web/components/diff/diff-utils.ts b/frontend/web/components/diff/diff-utils.ts index 5c1f96b66763..2f6cf4202410 100644 --- a/frontend/web/components/diff/diff-utils.ts +++ b/frontend/web/components/diff/diff-utils.ts @@ -215,8 +215,8 @@ export function getSegmentDiff( ): TSegmentDiff { const oldRules = oldSegment?.rules || [] const newRules = newSegment?.rules || [] - const oldString = getRulesDiff(oldRules, 0) - const newString = getRulesDiff(newRules, 0) + const oldString = getRulesDiff(oldRules, 0, '', oldRules[0]?.type) + const newString = getRulesDiff(newRules, 0, '', newRules[0]?.type) return { newString, oldString, totalChanges: newString === oldString ? 0 : 1 } } @@ -224,21 +224,25 @@ function getRulesDiff( rules: SegmentRule[], level: number, currentString = '', + parentType: SegmentRule['type'] = 'ALL', ): string { const indent = ' '.repeat(level) let str = currentString || `${indent}\n` + const connector = parentType === 'ANY' ? 'Or' : 'And' rules.forEach((rule, i) => { - const prepend = i > 0 ? 'And ' : '' + const prepend = i > 0 ? `${connector} ` : '' if (rule.type === 'ANY') { str += `${indent}------ ${prepend}Any of the following ------\n` + } else if (rule.type === 'ALL') { + str += `${indent}------ ${prepend}All of the following ------\n` } else if (rule.type === 'NONE') { str += `${indent}------ ${prepend}None of the following ------\n` } str = getConditionsDiff(rule.conditions, level + 1, str) if (rule.rules) { - str = getRulesDiff(rule.rules, level + 1, str) + str = getRulesDiff(rule.rules, level + 1, str, rule.type) } }) diff --git a/frontend/web/components/modals/CreateSegment.tsx b/frontend/web/components/modals/CreateSegment.tsx index aaca54def6ce..a4e4510a4698 100644 --- a/frontend/web/components/modals/CreateSegment.tsx +++ b/frontend/web/components/modals/CreateSegment.tsx @@ -229,6 +229,25 @@ const CreateSegment: FC = ({ setRules(newRules) } + const topLevelRuleType: 'ALL' | 'ANY' = + rules[0]?.type === 'ANY' ? 'ANY' : 'ALL' + const hasMixedTopLevelRuleTypes = + rules.length > 1 && rules.some((r) => r.type !== rules[0]?.type) + + const setTopLevelRuleType = (newType: 'ALL' | 'ANY') => { + const subRuleType = newType === 'ANY' ? 'ALL' : 'ANY' + const newRules = cloneDeep(rules) + for (const topRule of newRules) { + topRule.type = newType + topRule.rules = topRule.rules.map((subRule) => { + if (subRule.delete || subRule.type === 'NONE') return subRule + return { ...subRule, type: subRuleType as SegmentRule['type'] } + }) + } + setRules(newRules) + setValueChanged(true) + } + const save = async (e: FormEvent) => { try { Utils.preventDefault(e) @@ -405,6 +424,12 @@ const CreateSegment: FC = ({ const rulesEl = (
+ {hasMixedTopLevelRuleTypes && ( + + This segment has top-level rules with mixed types. Changing the rule + type will normalise all top-level rules to the same type. + + )}
{rules.map((topRule, rulesIndex) => topRule.rules.map((rule, i) => { @@ -414,7 +439,11 @@ const CreateSegment: FC = ({ const displayIndex = rulesToShow.indexOf(rule) return (
- + = ({ rule={rule} index={i} operators={operators} + conditionLabel={rule.type === 'ALL' ? 'And' : 'Or'} onChange={(v: SegmentRule) => { setValueChanged(true) updateRule(rulesIndex, i, v) @@ -438,27 +468,38 @@ const CreateSegment: FC = ({
{hasNoRules && ( - Add at least one AND/NOT rule to create a segment. + {topLevelRuleType === 'ANY' + ? 'Add at least one OR rule to create a segment.' + : 'Add at least one AND/NOT rule to create a segment.'} )} {!readOnly && ( -
addRule('ANY')} className='text-center'> +
+ addRule(topLevelRuleType === 'ANY' ? 'ALL' : 'ANY') + } + className='text-center' + > +
+ )} + {topLevelRuleType !== 'ANY' && ( +
addRule('NONE')} className='text-center'> +
)} -
addRule('NONE')} className='text-center'> - -
@@ -528,6 +569,8 @@ const CreateSegment: FC = ({ isValid={isValid} isLimitReached={isLimitReached} onCancel={onCancel} + topLevelRuleType={topLevelRuleType} + setTopLevelRuleType={setTopLevelRuleType} />
@@ -592,6 +635,8 @@ const CreateSegment: FC = ({ isValid={isValid} isLimitReached={isLimitReached} onCancel={onCancel} + topLevelRuleType={topLevelRuleType} + setTopLevelRuleType={setTopLevelRuleType} />
@@ -629,6 +674,8 @@ const CreateSegment: FC = ({ isValid={isValid} isLimitReached={isLimitReached} onCancel={onCancel} + topLevelRuleType={topLevelRuleType} + setTopLevelRuleType={setTopLevelRuleType} /> )} diff --git a/frontend/web/components/modals/CreateSegmentRulesTabForm.tsx b/frontend/web/components/modals/CreateSegmentRulesTabForm.tsx index 0fc0d8f6c9ea..0c9663be9b0f 100644 --- a/frontend/web/components/modals/CreateSegmentRulesTabForm.tsx +++ b/frontend/web/components/modals/CreateSegmentRulesTabForm.tsx @@ -4,6 +4,7 @@ import Input from 'components/base/forms/Input' import InputGroup from 'components/base/forms/InputGroup' import Switch from 'components/Switch' import Button from 'components/base/forms/Button' +import InlinePillToggle from 'components/base/forms/InlinePillToggle' import Format from 'common/utils/format' import Utils from 'common/utils/utils' import Constants from 'common/constants' @@ -45,6 +46,8 @@ interface CreateSegmentRulesTabFormProps { isValid: boolean isLimitReached: boolean onCancel?: () => void + topLevelRuleType?: 'ALL' | 'ANY' + setTopLevelRuleType?: (type: 'ALL' | 'ANY') => void } const CreateSegmentRulesTabForm: React.FC = ({ @@ -68,8 +71,10 @@ const CreateSegmentRulesTabForm: React.FC = ({ setDescription, setName, setShowDescriptions, + setTopLevelRuleType, setValueChanged, showDescriptions, + topLevelRuleType = 'ALL', }) => { const SEGMENT_ID_MAXLENGTH = Constants.forms.maxLength.SEGMENT_ID @@ -184,11 +189,6 @@ const CreateSegmentRulesTabForm: React.FC = ({ Show condition descriptions - - - {!condensed && ( Trait names are case sensitive. Learn more about rule and trait @@ -199,6 +199,33 @@ const CreateSegmentRulesTabForm: React.FC = ({ . )} + {!readOnly && + setTopLevelRuleType && + Utils.getFlagsmithHasFeature('segment_any_rule_type') ? ( + + + + + + ) : ( + + + + )} {allWarnings?.map((warning, i) => (
diff --git a/frontend/web/components/segments/Rule/Rule.tsx b/frontend/web/components/segments/Rule/Rule.tsx index 55b8f1cb34e3..20c4d5d41315 100644 --- a/frontend/web/components/segments/Rule/Rule.tsx +++ b/frontend/web/components/segments/Rule/Rule.tsx @@ -27,9 +27,11 @@ interface RuleProps { 'data-test'?: string errors: SegmentConditionsError[] projectId: number + conditionLabel?: string } const Rule: React.FC = ({ + conditionLabel, 'data-test': dataTest, errors, onChange, @@ -201,6 +203,7 @@ const Rule: React.FC = ({ rules={rules} data-test={`${dataTest}`} projectId={projectId} + conditionLabel={conditionLabel} /> ))}
diff --git a/frontend/web/components/segments/Rule/components/RuleConditionRow.tsx b/frontend/web/components/segments/Rule/components/RuleConditionRow.tsx index 6712bbce68e3..64baf1dc8013 100644 --- a/frontend/web/components/segments/Rule/components/RuleConditionRow.tsx +++ b/frontend/web/components/segments/Rule/components/RuleConditionRow.tsx @@ -31,10 +31,12 @@ interface RuleConditionRowProps { addRule: () => void rules: SegmentCondition[] projectId: number + conditionLabel?: string } const RuleConditionRow: React.FC = ({ addRule, + conditionLabel = 'Or', 'data-test': dataTest, errors: ruleErrors, operators, @@ -75,7 +77,7 @@ const RuleConditionRow: React.FC = ({
- Or + {conditionLabel}
@@ -141,7 +143,7 @@ const RuleConditionRow: React.FC = ({ type='button' onClick={addRule} > - Or + {conditionLabel} ) : (
diff --git a/frontend/web/styles/components/_index.scss b/frontend/web/styles/components/_index.scss index c0f41182d1d1..d8bdcfe21eea 100644 --- a/frontend/web/styles/components/_index.scss +++ b/frontend/web/styles/components/_index.scss @@ -17,3 +17,4 @@ @import 'feature-row-skeleton'; @import 'empty-state'; @import 'oauth-authorize'; +@import 'inline-pill-toggle'; diff --git a/frontend/web/styles/components/_inline-pill-toggle.scss b/frontend/web/styles/components/_inline-pill-toggle.scss new file mode 100644 index 000000000000..f5dba6288744 --- /dev/null +++ b/frontend/web/styles/components/_inline-pill-toggle.scss @@ -0,0 +1,62 @@ +.inline-pill-toggle { + display: inline-flex; + background-color: var(--color-surface-muted); + border-radius: var(--radius-lg); + vertical-align: middle; + + &--small { + padding: 2px; + gap: 1px; + } + + &--medium { + padding: 3px; + gap: 2px; + } + + &--large { + padding: 4px; + gap: 2px; + } + + &__option { + border: none; + border-radius: var(--radius-md); + background: transparent; + color: var(--color-text-secondary); + font-weight: 700; + font-family: inherit; + letter-spacing: 0.3px; + text-transform: uppercase; + cursor: pointer; + white-space: nowrap; + + &:focus-visible { + outline: 2px solid var(--color-border-action); + outline-offset: 2px; + } + + .inline-pill-toggle--small & { + padding: 0 6px; + font-size: $font-caption-xs; + line-height: 18px; + } + + .inline-pill-toggle--medium & { + padding: 1px 8px; + font-size: $font-caption-sm; + line-height: 20px; + } + + .inline-pill-toggle--large & { + padding: 2px 12px; + font-size: $font-caption; + line-height: $btn-line-height-sm; + } + + &--active { + background-color: var(--color-surface-action); + color: var(--slate-0); + } + } +}