diff --git a/frontend/e2e/helpers/e2e-helpers.playwright.ts b/frontend/e2e/helpers/e2e-helpers.playwright.ts index 85ded873745a..c60b3d348c9d 100644 --- a/frontend/e2e/helpers/e2e-helpers.playwright.ts +++ b/frontend/e2e/helpers/e2e-helpers.playwright.ts @@ -1,4 +1,4 @@ -import { Page, expect } from '@playwright/test'; +import { Locator, Page, expect } from '@playwright/test'; import { LONG_TIMEOUT, SHORT_TIMEOUT, byId, log, logUsingLastSection, getFlagsmith } from './utils.playwright'; // Re-export for backwards compatibility @@ -18,13 +18,33 @@ export type Rule = { export class E2EHelpers { constructor(private page: Page) {} + // The value editors are selected by role and accessible name rather than a + // data-test. The feature value label switches to "Control Value %" + // once the feature has variations, hence the alternation. + featureValueField(): Locator { + return this.page.getByRole('textbox', { name: /^(Value|Control Value)/ }); + } + + variationValueField(index: number): Locator { + return this.page.getByRole('textbox', { name: 'Variation Value' }).nth(index); + } + + // The override's own label is "Value", or "Segment Control Value" once the + // feature has variations. Anchored, because getByRole matches the name as a + // substring and the row also holds read-only "Variation Value" editors. + segmentOverrideValueField(index: number): Locator { + return this.page + .locator(byId(`segment-override-${index}`)) + .getByRole('textbox', { name: /^(Value|Segment Control Value)$/ }); + } + async isElementExists(selector: string): Promise { return await this.page.locator(byId(selector)).count() > 0; } - async setText(selector: string, text: string) { + async setText(selector: string | Locator, text: string) { logUsingLastSection(`Set text ${selector} : ${text}`); - const element = this.page.locator(selector).first(); + const element = typeof selector === 'string' ? this.page.locator(selector).first() : selector; await element.waitFor({ state: 'visible', timeout: LONG_TIMEOUT }); await element.clear(); if (text) { @@ -32,9 +52,10 @@ export class E2EHelpers { } } - async waitForElementVisible(selector: string, timeout: number = LONG_TIMEOUT) { + async waitForElementVisible(selector: string | Locator, timeout: number = LONG_TIMEOUT) { logUsingLastSection(`Waiting element visible ${selector}`); - await this.page.locator(selector).first().waitFor({ + const element = typeof selector === 'string' ? this.page.locator(selector).first() : selector; + await element.waitFor({ state: 'visible', timeout }); @@ -269,7 +290,7 @@ export class E2EHelpers { await featureRow.waitFor({ state: 'visible', timeout: LONG_TIMEOUT }); await featureRow.dispatchEvent('click'); await this.waitForElementVisible('#create-feature-modal'); - await this.waitForElementVisible(byId('featureValue')); + await this.waitForElementVisible(this.featureValueField()); } // Create a feature @@ -296,7 +317,7 @@ export class E2EHelpers { await this.gotoFeatures(); await this.click('#show-create-feature-btn'); await this.setText(byId('featureID'), name); - await this.setText(byId('featureValue'), `${value}`); + await this.setText(this.featureValueField(), `${value}`); await this.setText(byId('featureDesc'), description); if (!defaultOff) { await this.click(byId('toggle-feature-button')); @@ -305,7 +326,7 @@ export class E2EHelpers { const v = mvs[i]; await this.click(byId('add-variation')); await this.page.waitForTimeout(200); - await this.setText(byId(`featureVariationValue${i}`), v.value); + await this.setText(this.variationValueField(i), v.value); await this.setText(byId(`featureVariationWeight${v.value}`), `${v.weight}`); await this.page.waitForTimeout(100); } @@ -588,7 +609,7 @@ export class E2EHelpers { await this.click(byId('segment_overrides')); } await this.click(dropdownSelector); - await this.waitForElementVisible(byId(`segment-override-value-${index}`)); + await this.waitForElementVisible(this.segmentOverrideValueField(index)); } // Add segment override for boolean flags @@ -611,7 +632,7 @@ export class E2EHelpers { // Add segment override for remote configs async addSegmentOverrideConfig(index: number, value: string | number | boolean, selectionIndex: number = 0) { await this.openSegmentOverride(index, selectionIndex); - await this.setText(byId(`segment-override-value-${index}`), `${value}`); + await this.setText(this.segmentOverrideValueField(index), `${value}`); await this.click(byId(`segment-override-toggle-${index}`)); } @@ -631,7 +652,7 @@ export class E2EHelpers { await featureRow.dispatchEvent('click'); await this.waitForElementVisible(byId('update-feature-btn')); if (value !== '') { - await this.setText(byId('featureValue'), `${value}`); + await this.setText(this.featureValueField(), `${value}`); } if (mvs.length > 0) { await this.page.waitForTimeout(500); diff --git a/frontend/e2e/tests/change-request-test.pw.ts b/frontend/e2e/tests/change-request-test.pw.ts index f1b681acb30c..d3cdf16057e6 100644 --- a/frontend/e2e/tests/change-request-test.pw.ts +++ b/frontend/e2e/tests/change-request-test.pw.ts @@ -12,6 +12,7 @@ test.describe('Change Request Tests', () => { page, }, testInfo) => { const { + featureValueField, assertChangeRequestCount, approveChangeRequest, assertInputValue, @@ -73,7 +74,7 @@ test.describe('Change Request Tests', () => { log('Create change request by editing feature value') await gotoFeatures() await gotoFeature(featureName) - await setText(byId('featureValue'), 'updated_value') + await setText(featureValueField(), 'updated_value') await createChangeRequest( 'Update feature value', @@ -126,7 +127,7 @@ test.describe('Change Request Tests', () => { await page.reload({ waitUntil: 'domcontentloaded' }) await waitForElementVisible('#show-create-feature-btn') await gotoFeature(featureName) - await expect(page.locator(byId('featureValue'))).toHaveValue('updated_value', { timeout: 15000 }) + await expect(featureValueField()).toHaveText('updated_value', { timeout: 15000 }) await closeModal() log('Verify value via API') diff --git a/frontend/e2e/tests/mv-options-tests.pw.ts b/frontend/e2e/tests/mv-options-tests.pw.ts index 6cd540120f9f..c8d9cf718d8f 100644 --- a/frontend/e2e/tests/mv-options-tests.pw.ts +++ b/frontend/e2e/tests/mv-options-tests.pw.ts @@ -22,6 +22,7 @@ const variantCards = (page: Page) => page.locator('#create-feature-modal .varian test.describe('Multivariate Options', () => { test('Repeated saves keep the variant set stable @oss', async ({ page }) => { const { + variationValueField, closeModal, createRemoteConfig, editRemoteConfig, @@ -62,6 +63,7 @@ test.describe('Multivariate Options', () => { test('Variants can be added and removed in a single save @oss', async ({ page }) => { const { + variationValueField, click, closeModal, createRemoteConfig, @@ -90,7 +92,7 @@ test.describe('Multivariate Options', () => { await expect(variantCards(page)).toHaveCount(1); await click(byId('add-variation')); await page.waitForTimeout(200); - await setText(byId('featureVariationValue1'), 'added'); + await setText(variationValueField(1), 'added'); await page.waitForTimeout(500); await click(byId('update-feature-btn')); await waitForToast(); diff --git a/frontend/e2e/tests/segment-test.pw.ts b/frontend/e2e/tests/segment-test.pw.ts index e255a7aef2e8..7badc460c909 100644 --- a/frontend/e2e/tests/segment-test.pw.ts +++ b/frontend/e2e/tests/segment-test.pw.ts @@ -84,6 +84,7 @@ const segmentAnyRules = [ test('Segment test 1 - Create, update, and manage segments with multivariate flags @oss', async ({ page }, testInfo) => { const { + featureValueField, addSegmentOverride, assertInputValue, assertUserFeatureValue, @@ -204,6 +205,7 @@ test('Segment test 1 - Create, update, and manage segments with multivariate fla test('Segment test 2 - Test segment priority and overrides @oss', async ({ page }) => { const { + featureValueField, addSegmentOverride, addSegmentOverrideConfig, assertUserFeatureValue, @@ -312,6 +314,7 @@ test('Segment test 2 - Test segment priority and overrides @oss', async ({ page test('Segment test 3 - Test user-specific feature overrides @oss', async ({ page }, testInfo) => { const { + featureValueField, assertUserFeatureValue, click, clickUserFeature, @@ -350,7 +353,7 @@ test('Segment test 3 - Test user-specific feature overrides @oss', async ({ page log('Edit flag for user') await clickUserFeature(REMOTE_CONFIG_FEATURE) - await setText(byId('featureValue'), 'small') + await setText(featureValueField(), 'small') await click('#update-feature-btn') await waitAndRefresh() // wait and refresh to avoid issues with data sync from UK -> US in github workflows await assertUserFeatureValue(REMOTE_CONFIG_FEATURE, '"small"') @@ -393,6 +396,7 @@ test('Segment test 4 - Create ANY rule type segment and verify match changes whe const ANY_FEATURE = 'any_segment_feature' const ANY_SEGMENT = 'any_segment_test' const { + featureValueField, addSegmentOverrideConfig, assertUserFeatureValue, click, diff --git a/frontend/e2e/tests/versioning-tests.pw.ts b/frontend/e2e/tests/versioning-tests.pw.ts index f512c5c5553d..c266b6465447 100644 --- a/frontend/e2e/tests/versioning-tests.pw.ts +++ b/frontend/e2e/tests/versioning-tests.pw.ts @@ -10,6 +10,7 @@ import { E2E_USER, PASSWORD } from '../config'; test('Versioning tests - Create, edit, and compare feature versions @oss', async ({ page }, testInfo) => { const { + variationValueField, assertNumberOfVersions, click, closeModal, @@ -100,7 +101,7 @@ test('Versioning tests - Create, edit, and compare feature versions @oss', async await expect(page.locator(byId('featureVariationKey0'))).toHaveText('primary') await click(byId('add-variation')) await page.waitForTimeout(200) - await setText(byId('featureVariationValue2'), 'huge') + await setText(variationValueField(2), 'huge') await page.waitForTimeout(500) await click(byId('update-feature-btn')) await waitForToast() diff --git a/frontend/web/components/Highlight.js b/frontend/web/components/Highlight.js index 8c0dfa23739e..de6c241d852b 100644 --- a/frontend/web/components/Highlight.js +++ b/frontend/web/components/Highlight.js @@ -157,10 +157,12 @@ class Highlight extends React.Component { style={this.props.style} data-test={this.props['data-test']} aria-labelledby={this.props['aria-labelledby']} - // Without a role a contenteditable is announced as plain text, and - // aria-labelledby has nothing to name. - role={this.props.onChange ? 'textbox' : undefined} - aria-multiline={this.props.onChange ? true : undefined} + // Set by the caller: a value field wants role=textbox so its label + // names it, while the code blocks that also use Highlight are not + // form controls and pass nothing. + role={this.props.role} + aria-readonly={this.props['aria-readonly']} + aria-multiline={this.props.role === 'textbox' ? true : undefined} contentEditable={!!this.props.onChange} onBlur={this.onBlur} onFocus={this.onFocus} diff --git a/frontend/web/components/SegmentOverrides.js b/frontend/web/components/SegmentOverrides.js index 28cf75567995..5fde8a43bc9d 100644 --- a/frontend/web/components/SegmentOverrides.js +++ b/frontend/web/components/SegmentOverrides.js @@ -277,10 +277,8 @@ const SegmentOverrideInner = class Override extends React.Component {
} value={v.value} - data-test={`segment-override-value-${index}`} - placeholder="Value e.g. 'big' " disabled={readOnly} onChange={ readOnly diff --git a/frontend/web/components/ValueEditor/ValueEditor.tsx b/frontend/web/components/ValueEditor/ValueEditor.tsx index 60bac9ee43be..5826f450b020 100644 --- a/frontend/web/components/ValueEditor/ValueEditor.tsx +++ b/frontend/web/components/ValueEditor/ValueEditor.tsx @@ -22,7 +22,6 @@ import './ValueEditor.scss' export interface ValueEditorProps { className?: string - 'data-test'?: string disabled?: boolean // Renders the field's label and wires it to the editor. Callers used to // render their own, which is why three different label treatments grew up @@ -34,17 +33,11 @@ export interface ValueEditorProps { labelAfter?: ReactNode labelTooltip?: string language?: ValueEditorLanguage - name?: string onBlur?: () => void // The edited text. Deliberately a string, not FlagsmithValue: this edits // text, and deciding that "123" is a number is Flagsmith's domain logic. // Callers interpret it (Utils.getTypedValue, Utils.valueToFeatureState). onChange?: (value: string) => void - // placeholder and readOnly only reach the editor under E2E, which swaps - // Highlight for a plain textarea. Highlight renders its own - // 'Enter a value...' and stops accepting input while disabled. - placeholder?: string - readOnly?: boolean // Fires when the value stops or starts parsing under the active format. onValidityChange?: (error: string | false) => void value?: FlagsmithValue @@ -57,14 +50,10 @@ const ValueEditor: FC = ({ labelAfter, labelTooltip, language: languageProp, - name, onBlur, onChange, - placeholder, - readOnly, onValidityChange, value, - ...rest }) => { const [language, setLanguage] = useState( languageProp ?? 'txt', @@ -137,30 +126,17 @@ const ValueEditor: FC = ({
{showControls && } - {E2E ? ( -