Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
80f872b
refactor(value-editor): migrate to TypeScript and split by concern
talissoncosta Sep 2, 2026
bf8eb6c
fix(value-editor): read XML parser errors in Firefox
talissoncosta Sep 2, 2026
208388c
test(value-editor): cover value validation and clipboard copy
talissoncosta Sep 2, 2026
083dfa8
fix(a11y): make the format labels real buttons
talissoncosta Sep 2, 2026
dc41b28
refactor(value-editor): own the label and name the editor
talissoncosta Sep 2, 2026
e5ae34c
refactor(saml): use InputGroup for the IdP metadata field
talissoncosta Sep 2, 2026
754fa04
refactor(mv): share the control value weight chip
talissoncosta Sep 2, 2026
3196173
refactor(value-editor): move its styles out of the highlight.js overr…
talissoncosta Sep 2, 2026
3f784ae
fix(value-editor): detect the format when the value arrives, not at m…
talissoncosta Sep 2, 2026
3fbcf6c
refactor(value-editor): own validity instead of the format row
talissoncosta Sep 2, 2026
8f42bd3
refactor(value-editor): use utilities for layout and colour
talissoncosta Sep 7, 2026
f97118f
fix(value-editor): show the validation icon in its own colour
talissoncosta Sep 7, 2026
18ac82c
refactor(value-editor): name the validation error and tone types
talissoncosta Sep 7, 2026
a2878a8
fix(a11y): show a focus border on the value editor
talissoncosta Sep 7, 2026
d500dc5
style(value-editor): blank line before the focus comment
talissoncosta Sep 7, 2026
6af00b1
refactor(value-editor): drop the E2E textarea fork
talissoncosta Sep 2, 2026
dfad5df
test(e2e): select the value editors by role and name
talissoncosta Sep 2, 2026
8adcbe0
fix(review): tighten the E2E selector, Highlight updates and SCSS import
talissoncosta Sep 7, 2026
ea90fdb
test(e2e): cover format validation in the browser
talissoncosta Sep 7, 2026
eb04dff
refactor(saml): keep the highlighted XML editor, pin it with language
talissoncosta Sep 7, 2026
41d8eba
fix(review): tighten the contract and the format row's markup
talissoncosta Sep 8, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions frontend/common/utils/__tests__/copyToClipboard.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { copyToClipboard } from 'common/utils/copyToClipboard'

const writeText = jest.fn()
const toast = jest.fn()

beforeEach(() => {
writeText.mockReset().mockResolvedValue(undefined)
toast.mockReset()
;(global as any).toast = toast
Object.defineProperty(global, 'navigator', {
configurable: true,
value: { clipboard: { writeText } },
writable: true,
})
})

describe('copyToClipboard', () => {
it('writes the value and toasts the default success message', async () => {
await copyToClipboard('DEFAULT_VALUE')

expect(writeText).toHaveBeenCalledWith('DEFAULT_VALUE')
expect(toast).toHaveBeenCalledWith('Copied to clipboard')
})

it('toasts a caller-supplied success message instead', async () => {
await copyToClipboard('prompt', 'Cleanup prompt copied to clipboard')

expect(toast).toHaveBeenCalledWith('Cleanup prompt copied to clipboard')
})

it('toasts the failure and rethrows when the write is rejected', async () => {
const error = new Error('denied')
writeText.mockRejectedValue(error)

await expect(copyToClipboard('value')).rejects.toThrow(error)
expect(toast).toHaveBeenCalledWith('Failed to copy to clipboard')
})

it('toasts a caller-supplied failure message instead', async () => {
writeText.mockRejectedValue(new Error('denied'))

await expect(
copyToClipboard('value', undefined, 'Could not copy the value'),
).rejects.toThrow()
expect(toast).toHaveBeenCalledWith('Could not copy the value')
})
})
21 changes: 21 additions & 0 deletions frontend/common/utils/copyToClipboard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* Write `value` to the clipboard and toast the outcome.
*
* Rethrows after toasting so callers that need to react to a failure can,
* but the toast means most callers do not have to.
*/
export const copyToClipboard = async (
value: string,
successMessage?: string,
errorMessage?: string,
) => {
try {
await navigator.clipboard.writeText(value)
toast(successMessage ?? 'Copied to clipboard')
} catch (error) {
toast(errorMessage ?? 'Failed to copy to clipboard')
throw error
}
}

export default copyToClipboard
15 changes: 2 additions & 13 deletions frontend/common/utils/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export const planNames = {
startup: 'Startup',
}
import BaseUtils from './base/_utils'
import { copyToClipboard } from './copyToClipboard'
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const Utils = Object.assign({}, BaseUtils, {
appendImage: (src: string) => {
const img = document.createElement('img')
Expand Down Expand Up @@ -148,19 +149,7 @@ const Utils = Object.assign({}, BaseUtils, {
return res
},

copyToClipboard: async (
value: string,
successMessage?: string,
errorMessage?: string,
) => {
try {
await navigator.clipboard.writeText(value)
toast(successMessage ?? 'Copied to clipboard')
} catch (error) {
toast(errorMessage ?? 'Failed to copy to clipboard')
throw error
}
},
copyToClipboard,

displayLimitAlert(type: string, percentage: number | undefined) {
const envOrProject =
Expand Down
94 changes: 77 additions & 17 deletions frontend/documentation/components/ValueEditor.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import React, { useState } from 'react'
import React, { useEffect, useState } from 'react'
import type { Meta, StoryObj } from 'storybook'

import ValueEditor from 'components/ValueEditor'
import FieldLabel from 'components/base/forms/FieldLabel'
import Constants from 'common/constants'
import ValueEditor, { ValueEditorProps } from 'components/ValueEditor'
import ControlWeightChip from 'components/mv/ControlWeightChip'

const meta: Meta = {
parameters: { chromatic: { disableSnapshot: false } },
Expand All @@ -13,23 +13,27 @@ export default meta

type Story = StoryObj

const DEFAULT_TOOLTIP = Constants.strings.REMOTE_CONFIG_DESCRIPTION
// The real props, so a story cannot drift from the component's contract.
type InteractiveProps = Omit<ValueEditorProps, 'value' | 'onChange'> & {
initialValue?: string
width?: number
}

const Interactive = ({
initialValue = '',
label,
tooltip = DEFAULT_TOOLTIP,
width = 640,
...props
}: Record<string, any>) => {
}: InteractiveProps) => {
const [value, setValue] = useState(initialValue)
return (
<div style={{ maxWidth: 640, paddingTop: 24 }}>
{label && <FieldLabel tooltip={tooltip}>{label}</FieldLabel>}
<div style={{ maxWidth: width, padding: 16 }}>
<ValueEditor {...props} value={value} onChange={setValue} />
</div>
)
}

// Empty state. The "Enter a value..." text is not a real ::placeholder — it is
// rendered into the contenteditable and styled by `code.txt.empty`.
export const Default: Story = {
render: () => <Interactive label='Value' />,
}
Expand All @@ -53,23 +57,49 @@ export const Json: Story = {
render: () => (
<Interactive
label='Value'
language='json'
initialValue='{ "colour": "blue", "size": 12 }'
/>
),
}

// A value that arrives after mount, the way a loaded feature does. Detection
// has to wait for it: a mount-only check left JSON rendering as .txt.
const LateLoading = () => {
const [value, setValue] = useState<string>('')
useEffect(() => {
const timer = setTimeout(() => setValue('{ "colour": "blue" }'), 150)
return () => clearTimeout(timer)
}, [])
return (
<div style={{ maxWidth: 640, padding: 16 }}>
<ValueEditor label='Value' value={value} onChange={setValue} />
</div>
)
}

export const ValueArrivesAfterMount: Story = {
render: () => <LateLoading />,
}

// The danger tone, which no other story shows. The format has to be chosen
// rather than pinned: a pinned editor has no format row to render the warning
// against.
export const InvalidJson: Story = {
render: () => (
<Interactive label='Value' language='json' initialValue='{ "colour": ' />
),
play: async ({ canvasElement }: { canvasElement: HTMLElement }) => {
const buttons = canvasElement.querySelectorAll('.select-language button')
const json = Array.from(buttons).find(
(button) => button.textContent?.trim() === '.json',
)
;(json as HTMLButtonElement | undefined)?.click()
},
render: () => <Interactive label='Value' initialValue='{ "colour": ' />,
}

export const CodeMedium: Story = {
render: () => (
<Interactive
label='Variation Value'
tooltip={Constants.strings.REMOTE_CONFIG_DESCRIPTION_VARIATION}
labelTooltip={Constants.strings.REMOTE_CONFIG_DESCRIPTION_VARIATION}
className='code-medium'
initialValue='variant-a'
/>
Expand All @@ -82,13 +112,43 @@ export const Disabled: Story = {
),
}

export const OnlyOneLang: Story = {
// A pinned format hides the row, since there is nothing to switch to. This is
// what the SAML IdP metadata field renders.
export const XmlOnly: Story = {
render: () => (
<Interactive
label='IDP metadata XML'
onlyOneLang
label='IdP metadata XML'
language='xml'
initialValue={'<EntityDescriptor entityID="https://example.com" />'}
/>
),
}

// The multivariate control value carries a weight chip and a tooltip, so it is
// the widest label this component gets. Label and format buttons share one flex
// row, so they compress rather than overlap.
const controlWeight = <ControlWeightChip percentage={100} />

export const BadgeLabel: Story = {
render: () => (
<Interactive
label='Control Value'
labelAfter={controlWeight}
labelTooltip={Constants.strings.REMOTE_CONFIG_DESCRIPTION_VARIATION}
initialValue='DEFAULT_VALUE'
/>
),
}

// The same label at the narrowest width the drawer reaches.
export const BadgeLabelNarrow: Story = {
render: () => (
<Interactive
label='Control Value'
labelAfter={controlWeight}
labelTooltip={Constants.strings.REMOTE_CONFIG_DESCRIPTION_VARIATION}
initialValue='DEFAULT_VALUE'
width={380}
/>
),
}
49 changes: 38 additions & 11 deletions frontend/e2e/helpers/e2e-helpers.playwright.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { Page, expect } from '@playwright/test';
import { Locator, Page, expect } from '@playwright/test';

// A CSS/data-test string, or a Locator built from role and accessible name.
type SelectorOrLocator = string | Locator;
import { LONG_TIMEOUT, SHORT_TIMEOUT, byId, log, logUsingLastSection, getFlagsmith } from './utils.playwright';

// Re-export for backwards compatibility
Expand All @@ -18,23 +21,47 @@ 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 label reads "Control Value" once the feature has variations,
// hence the alternation; the weight chip is a labelAfter sibling, so it stays
// out of the accessible name.
featureValueField(): Locator {
return this.page
.locator('#create-feature-modal')
.getByRole('textbox', { name: /^(Value|Control Value)$/ });
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

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<boolean> {
return await this.page.locator(byId(selector)).count() > 0;
}

async setText(selector: string, text: string) {
async setText(selector: SelectorOrLocator, 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) {
await element.fill(text);
}
}

async waitForElementVisible(selector: string, timeout: number = LONG_TIMEOUT) {
async waitForElementVisible(selector: SelectorOrLocator, 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
});
Expand Down Expand Up @@ -269,7 +296,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
Expand All @@ -296,7 +323,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'));
Expand All @@ -305,7 +332,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);
}
Expand Down Expand Up @@ -588,7 +615,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
Expand All @@ -611,7 +638,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}`));
}

Expand All @@ -631,7 +658,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);
Expand Down
8 changes: 4 additions & 4 deletions frontend/e2e/tests/change-request-test.pw.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ test.describe('Change Request Tests', () => {
page,
}, testInfo) => {
const {
featureValueField,
assertChangeRequestCount,
approveChangeRequest,
assertInputValue,
closeModal,
closeModal,
createChangeRequest,
createEnvironment,
createRemoteConfig,
Expand Down Expand Up @@ -73,7 +73,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',
Expand Down Expand Up @@ -126,7 +126,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')
Expand Down
Loading
Loading