From acd2b7043df5ae92782cc8350ac3839cdeb29828 Mon Sep 17 00:00:00 2001 From: Victor Musienko <8777372+sdwvit@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:21:41 +0100 Subject: [PATCH 1/6] [AG-17044] Test(advanced-filter): bigint custom parser coverage and formatter docs (#14648) * [AG-17044] Test(advanced-filter): bigint custom parser coverage and formatter docs Adds regression coverage for the QA reproduction paths and documents `bigintFormatter`, which had no prose documentation. - Advanced Filter: a decimal operand entered in the Filter Builder is stored canonically, matches, and is displayed through the column's `bigintFormatter`. - BigInt Filter: hex `from`/`to` values are both parsed for a range condition. - Docs: `bigintFormatter` is the inverse of `bigintParser`; the model stores a canonical decimal string, so entered values are echoed back in the formatter's format in the floating filter and the Advanced Filter. * [AG-17044] Test(advanced-filter): assert builder value pill formatting before apply * [AG-17044] Fix(advanced-filter): open builder value editor with the formatted bigint operand The builder value editor seeded its input from the raw model value, so a bigint operand stored as canonical decimal replaced the formatted text shown on the pill as soon as the editor opened. Edit the displayed text instead for bigint columns, whose bigintFormatter output the column's parser accepts. * [AG-17044] Fix(advanced-filter): keep the typed bigint operand syntax in the builder The builder rebuilt its pills from the canonical model value, so a bigint operand typed in a syntax only the column's bigintParser understands showed as decimal while the filter text bar still showed what was typed. Seed the builder from the expression parser's operand text so the two surfaces agree across an apply. * [AG-17044] Refactor(advanced-filter): gate builder operand editing on a data type capability Replace the two hardcoded bigint comparisons with a single isOperandModelValueEditable predicate on AdvancedFilterExpressionService, and take forBuilder off the bean's getModel in favour of a named getBuilderModel. --- .../content/docs/filter-advanced/index.mdoc | 2 +- .../src/content/docs/filter-bigint/index.mdoc | 2 + .../advancedFilterExpressionService.ts | 14 ++ .../advancedFilter/advancedFilterService.ts | 14 +- .../builder/advancedFilterBuilderComp.ts | 2 +- .../builder/advancedFilterBuilderItemComp.ts | 3 +- .../builder/conditionPillWrapperComp.ts | 10 +- .../builder/iAdvancedFilterBuilder.ts | 2 + .../advancedFilter/builder/inputPillComp.ts | 6 +- .../colFilterExpressionParser.ts | 15 +- .../advancedFilter/filterExpressionParser.ts | 4 +- .../joinFilterExpressionParser.ts | 6 +- ...vanced-filter-bigint-custom-parser.test.ts | 130 ++++++++++++++++++ .../bigint-filter-custom-parser.test.ts | 36 +++++ .../filters/advancedFilterBuilderHarness.ts | 11 +- 15 files changed, 240 insertions(+), 17 deletions(-) diff --git a/documentation/ag-grid-docs/src/content/docs/filter-advanced/index.mdoc b/documentation/ag-grid-docs/src/content/docs/filter-advanced/index.mdoc index 8be328b2673..e04e74647f4 100644 --- a/documentation/ag-grid-docs/src/content/docs/filter-advanced/index.mdoc +++ b/documentation/ag-grid-docs/src/content/docs/filter-advanced/index.mdoc @@ -275,7 +275,7 @@ All of the [Cell Data Types](./cell-data-types) are supported in the Advanced Fi - **Text** - The value in the input is compared against the cell value before any [Value Formatters](./value-formatters/) are applied (similar to the [Text Filter](./filter-text/)). To change the value being compared against, a [Filter Value Getter](./filter-text/#text-filter-values) can be used. - **Number** - The value in the input is compared against the cell value (like in the [Number Filter](./filter-number/)). -- **BigInt** - The value in the input is parsed as a `bigint` (decimal integer syntax only, optional trailing `n`) and compared against the cell value (like in the [BigInt Filter](./filter-bigint/)). A column's [`bigintParser`](./filter-bigint/#custom-parsing) is used here too, so custom formats such as hexadecimal are also accepted. +- **BigInt** - The value in the input is parsed as a `bigint` (decimal integer syntax only, optional trailing `n`) and compared against the cell value (like in the [BigInt Filter](./filter-bigint/)). A column's [`bigintParser`](./filter-bigint/#custom-parsing) is used here too, so custom formats such as hexadecimal are also accepted, and its `bigintFormatter` is used to display a stored operand in the filter expression and the Filter Builder. - **Boolean** - No values are displayed for booleans as the filter option is used instead. - **Date** and **Date Time** - The value in the input is converted to a `Date` via the [Value Parser](./value-parsers/#value-parser). - **Date String** and **Date Time String** - The value in the input is converted to a `Date` using the [Value Parser](./value-parsers/#value-parser) and the [Date Parser](./cell-data-types/#date-as-string). This is compared against the cell values, which are also converted using the Date Parser. diff --git a/documentation/ag-grid-docs/src/content/docs/filter-bigint/index.mdoc b/documentation/ag-grid-docs/src/content/docs/filter-bigint/index.mdoc index 1a1da04627d..a5bb780c95e 100644 --- a/documentation/ag-grid-docs/src/content/docs/filter-bigint/index.mdoc +++ b/documentation/ag-grid-docs/src/content/docs/filter-bigint/index.mdoc @@ -46,6 +46,8 @@ The BigInt Filter accepts decimal integer syntax only: To accept other formats, such as hexadecimal, provide a `bigintParser` that converts the entered text to a `bigint` (return `null` for values it cannot parse). Pair it with `allowedCharPattern` so the extra characters can be typed into the filter input. The parsed value is what gets applied to filtering, and the same parser is used by the [Advanced Filter](./filter-advanced/) for `bigint` operands. +The filter model always stores the parsed value as a canonical decimal string, so provide a `bigintFormatter` — the inverse of the parser — to display stored values back in your own format. It is used by the [Floating Filter](./floating-filters/) and by the [Advanced Filter](./filter-advanced/) when displaying an operand, which means an entered value is echoed back in the formatter's format rather than exactly as typed. + ```{% frameworkTransform=true %} const gridOptions = { columnDefs: [ diff --git a/packages/ag-grid-enterprise/src/advancedFilter/advancedFilterExpressionService.ts b/packages/ag-grid-enterprise/src/advancedFilter/advancedFilterExpressionService.ts index fd7880d79e9..1667bce2cec 100644 --- a/packages/ag-grid-enterprise/src/advancedFilter/advancedFilterExpressionService.ts +++ b/packages/ag-grid-enterprise/src/advancedFilter/advancedFilterExpressionService.ts @@ -168,6 +168,20 @@ export class AdvancedFilterExpressionService extends BeanStub implements NamedBe return this.operandModelValueGetters[baseCellDataType](operand, column, baseCellDataType); } + /** + * Whether a stored operand is itself valid input for its data type, i.e. whether feeding the model + * value back into the expression or the builder editor yields the same value again. + * + * True for most types: text and number model values are their input form, and dates store the iso + * string the editor expects. It is false only for `bigint`, where the model holds the canonical + * decimal while input goes through the column's `bigintParser` - so a parser reading a non-decimal + * syntax would reinterpret that decimal as a different number. Those operands have to be presented + * through `getOperandDisplayValue` (the `bigintFormatter`) or kept as the text the user typed. + */ + public isOperandModelValueEditable(baseCellDataType: BaseCellDataType): boolean { + return baseCellDataType !== 'bigint'; + } + public getOperandDisplayValue(model: ColumnAdvancedFilterModel, skipFormatting?: boolean): string { const { filter, filterType } = model as Exclude; diff --git a/packages/ag-grid-enterprise/src/advancedFilter/advancedFilterService.ts b/packages/ag-grid-enterprise/src/advancedFilter/advancedFilterService.ts index 2f61b0d7221..f900e9f9526 100644 --- a/packages/ag-grid-enterprise/src/advancedFilter/advancedFilterService.ts +++ b/packages/ag-grid-enterprise/src/advancedFilter/advancedFilterService.ts @@ -94,9 +94,21 @@ export class AdvancedFilterService extends BeanStub implements NamedBean, IAdvan } public getModel(): AdvancedFilterModel | null { + return this.parseAppliedExpressionModel(false); + } + + /** + * As `getModel`, but with operands in the form the builder should edit rather than canonical model + * form, so a value typed in a syntax the model cannot reproduce survives the round-trip. + */ + public getBuilderModel(): AdvancedFilterModel | null { + return this.parseAppliedExpressionModel(true); + } + + private parseAppliedExpressionModel(forBuilder: boolean): AdvancedFilterModel | null { const expressionParser = this.createExpressionParser(this.appliedExpression); expressionParser?.parseExpression(); - return expressionParser?.getModel() ?? null; + return expressionParser?.getModel(forBuilder) ?? null; } public setModel(model: AdvancedFilterModel | null): void { diff --git a/packages/ag-grid-enterprise/src/advancedFilter/builder/advancedFilterBuilderComp.ts b/packages/ag-grid-enterprise/src/advancedFilter/builder/advancedFilterBuilderComp.ts index a54db858321..cd4ab9f35dd 100644 --- a/packages/ag-grid-enterprise/src/advancedFilter/builder/advancedFilterBuilderComp.ts +++ b/packages/ag-grid-enterprise/src/advancedFilter/builder/advancedFilterBuilderComp.ts @@ -290,7 +290,7 @@ export class AdvancedFilterBuilderComp extends Component; const key = (typeof filter === 'number' || typeof filter === 'bigint' ? _toStringOrNull(filter) : filter) ?? ''; + const valueFormatter = (value: string) => + this.advFilterExpSvc.getOperandDisplayValue({ ...this.filterModel, filter: value } as any, true); this.eOperandPill = this.createPill({ key, // Convert from the input format to display format. // Input format matches model format except for numbers, but these get stringified anyway - valueFormatter: (value) => - this.advFilterExpSvc.getOperandDisplayValue({ ...this.filterModel, filter: value } as any, true), + valueFormatter, + // Where the stored operand is not valid input for its type, edit the displayed text instead: + // the display value is produced by the same formatter whose output that type's parser accepts. + editValueFormatter: this.advFilterExpSvc.isOperandModelValueEditable(this.baseCellDataType) + ? undefined + : valueFormatter, baseCellDataType: this.baseCellDataType, cssClass: 'ag-advanced-filter-builder-value-pill', isSelect: false, diff --git a/packages/ag-grid-enterprise/src/advancedFilter/builder/iAdvancedFilterBuilder.ts b/packages/ag-grid-enterprise/src/advancedFilter/builder/iAdvancedFilterBuilder.ts index ea4611257bc..2ebf87050d4 100644 --- a/packages/ag-grid-enterprise/src/advancedFilter/builder/iAdvancedFilterBuilder.ts +++ b/packages/ag-grid-enterprise/src/advancedFilter/builder/iAdvancedFilterBuilder.ts @@ -27,6 +27,8 @@ export type CreatePillParams = CreateInputPillParams | CreateSelectPillParams; interface CreateInputPillParams extends BaseCreatePillParams { isSelect: false; valueFormatter: (value: string) => string; + /** Converts the stored value into the text the editor opens with. Defaults to the stored value. */ + editValueFormatter?: (value: string) => string; baseCellDataType: BaseCellDataType; } diff --git a/packages/ag-grid-enterprise/src/advancedFilter/builder/inputPillComp.ts b/packages/ag-grid-enterprise/src/advancedFilter/builder/inputPillComp.ts index 0218e2c3387..d685d740694 100644 --- a/packages/ag-grid-enterprise/src/advancedFilter/builder/inputPillComp.ts +++ b/packages/ag-grid-enterprise/src/advancedFilter/builder/inputPillComp.ts @@ -79,6 +79,7 @@ export class InputPillComp extends Component { private readonly params: { value: string; valueFormatter: (value: string) => string; + editValueFormatter?: (value: string) => string; cssClass: string; type: BaseCellDataType; ariaLabel: string; @@ -128,7 +129,10 @@ export class InputPillComp extends Component { } _setDisplayed(this.ePill, false); this.eEditor = this.createEditorComp(this.params.type); - this.eEditor.setValue(this.value); + const { editValueFormatter } = this.params; + // Edit the value as it is displayed, so a formatted operand does not flip back to the raw + // model value when the editor opens. + this.eEditor.setValue(editValueFormatter?.(this.value) ?? this.value); const eEditorGui = this.eEditor.getGui(); this.eEditor.addManagedElementListeners(eEditorGui, { keydown: (event: KeyboardEvent) => { diff --git a/packages/ag-grid-enterprise/src/advancedFilter/colFilterExpressionParser.ts b/packages/ag-grid-enterprise/src/advancedFilter/colFilterExpressionParser.ts index df3b3719916..cda76fa88dc 100644 --- a/packages/ag-grid-enterprise/src/advancedFilter/colFilterExpressionParser.ts +++ b/packages/ag-grid-enterprise/src/advancedFilter/colFilterExpressionParser.ts @@ -275,6 +275,16 @@ class OperandParser implements Parser { return this.modelValue; } + /** + * The operand the builder should present and edit: the model value where that is valid input, + * otherwise the text the user typed, which the model value cannot be turned back into. + */ + public getBuilderValue(): string | number { + return this.params.advFilterExpSvc.isOperandModelValueEditable(this.baseCellDataType) + ? this.modelValue + : this.operand; + } + private parseOperand(fromComplete: boolean, position: number): void { const { advFilterExpSvc } = this.params; this.endPosition = position; @@ -500,7 +510,7 @@ export class ColFilterExpressionParser { return null; } - public getModel(): AdvancedFilterModel { + public getModel(forBuilder?: boolean): AdvancedFilterModel { const colId = this.columnParser!.getColId(); const model = { filterType: this.columnParser!.baseCellDataType, @@ -508,7 +518,8 @@ export class ColFilterExpressionParser { type: this.operatorParser!.getOperatorKey(), }; if (this.operatorParser!.expectedNumOperands) { - (model as any).filter = this.operandParser!.getModelValue(); + const operandParser = this.operandParser!; + (model as any).filter = forBuilder ? operandParser.getBuilderValue() : operandParser.getModelValue(); } return model as AdvancedFilterModel; } diff --git a/packages/ag-grid-enterprise/src/advancedFilter/filterExpressionParser.ts b/packages/ag-grid-enterprise/src/advancedFilter/filterExpressionParser.ts index 811a9e77e2c..95cda2a3c7b 100644 --- a/packages/ag-grid-enterprise/src/advancedFilter/filterExpressionParser.ts +++ b/packages/ag-grid-enterprise/src/advancedFilter/filterExpressionParser.ts @@ -59,8 +59,8 @@ export class FilterExpressionParser { return this.joinExpressionParser.updateExpression(position, updateEntry, type)!; } - public getModel(): AdvancedFilterModel | null { - return this.isValid() ? this.joinExpressionParser.getModel() : null; + public getModel(forBuilder?: boolean): AdvancedFilterModel | null { + return this.isValid() ? this.joinExpressionParser.getModel(forBuilder) : null; } private createFunctionParams(): FilterExpressionFunctionParams { diff --git a/packages/ag-grid-enterprise/src/advancedFilter/joinFilterExpressionParser.ts b/packages/ag-grid-enterprise/src/advancedFilter/joinFilterExpressionParser.ts index 41b04476832..2fabcf3db96 100644 --- a/packages/ag-grid-enterprise/src/advancedFilter/joinFilterExpressionParser.ts +++ b/packages/ag-grid-enterprise/src/advancedFilter/joinFilterExpressionParser.ts @@ -360,15 +360,15 @@ export class JoinFilterExpressionParser { return updatedExpression; } - public getModel(): AdvancedFilterModel { + public getModel(forBuilder?: boolean): AdvancedFilterModel { if (this.expressionParsers.length > 1) { return { filterType: 'join', type: this.operatorParser.getModel(), - conditions: this.expressionParsers.map((parser) => parser.getModel()), + conditions: this.expressionParsers.map((parser) => parser.getModel(forBuilder)), }; } else { - return this.expressionParsers[0].getModel(); + return this.expressionParsers[0].getModel(forBuilder); } } diff --git a/testing/behavioural/src/filters/advanced-filter/advanced-filter-bigint-custom-parser.test.ts b/testing/behavioural/src/filters/advanced-filter/advanced-filter-bigint-custom-parser.test.ts index 972562bfe0d..777320af5b6 100644 --- a/testing/behavioural/src/filters/advanced-filter/advanced-filter-bigint-custom-parser.test.ts +++ b/testing/behavioural/src/filters/advanced-filter/advanced-filter-bigint-custom-parser.test.ts @@ -196,4 +196,134 @@ describe('Advanced Filter - bigint custom parser and formatter', () => { └── LEAF id:2 value:"1000n" `); }); + + test('builder decimal operand is stored canonically, matches, and displays via the formatter', async () => { + const api = gridsManager.createGrid('grid4', { + columnDefs: withParser, + rowData: [{ value: 10n }, { value: 255n }, { value: 1000n }, { value: 65535n }], + enableAdvancedFilter: true, + }); + await asyncSetTimeout(0); + api.setAdvancedFilterModel({ filterType: 'bigint', colId: 'value', type: 'equals', filter: '1000' } as any); + await asyncSetTimeout(0); + + const builder = await AdvancedFilterBuilderHarness.open(api); + const [condition] = await builder.conditionItems(); + + // The builder editor presents the stored operand through the formatter, not as the canonical + // decimal it is stored as — assert that here, while the builder is still open, so a + // regression showing `1000`/`255` in the editor cannot hide behind the applied expression. + expect(valuePillText(condition)).toBe('0x3E8'); + + await builder.setValue(condition, '255'); + // Re-query: committing the edit re-renders the condition row, detaching the earlier element. + const [editedCondition] = await builder.conditionItems(); + expect(valuePillText(editedCondition)).toBe('0xFF'); + + await builder.apply(); + await asyncSetTimeout(0); + + expect(api.getAdvancedFilterModel()).toEqual({ + filterType: 'bigint', + colId: 'value', + type: 'equals', + filter: '255', + }); + await new GridRows(api, 'builder decimal operand matches only 255').check(` + ROOT id:ROOT_NODE_ID + └── LEAF id:1 value:"255n" + `); + // The formatter is the canonical presentation of a stored operand, whatever syntax was typed. + expect(getService(api).getExpressionDisplayValue()).toBe('[Value] = 0xFF'); + }); + + test('builder value editor opens with the formatted operand, not the underlying decimal', async () => { + const api = gridsManager.createGrid('grid5', { + columnDefs: withParser, + rowData: [{ value: 10n }, { value: 255n }, { value: 1000n }], + enableAdvancedFilter: true, + }); + await asyncSetTimeout(0); + api.setAdvancedFilterModel({ filterType: 'bigint', colId: 'value', type: 'equals', filter: '1000' } as any); + await asyncSetTimeout(0); + + const builder = await AdvancedFilterBuilderHarness.open(api); + const [condition] = await builder.conditionItems(); + expect(valuePillText(condition)).toBe('0x3E8'); + + // Editing must start from the value shown on the pill (the formatter's output, which the + // column's parser accepts), not the canonical decimal the model stores. + expect((await builder.openValueEditor(condition)).value).toBe('0x3E8'); + }); + + test('builder keeps the typed operand syntax across apply when no formatter can reproduce it', async () => { + const api = gridsManager.createGrid('grid7', { + columnDefs: [ + { + field: 'value', + headerName: 'Value', + cellDataType: 'bigint', + filter: 'agBigIntColumnFilter', + filterParams: { allowedCharPattern: 'n0-9a-fA-FxX+\\-', bigintParser: parseBigInt }, + }, + ], + rowData: [{ value: 10n }, { value: 255n }, { value: 1000n }], + enableAdvancedFilter: true, + }); + await asyncSetTimeout(0); + const gridDiv = getGridElement(api)! as HTMLElement; + + // Without a bigintFormatter the model's canonical `255` cannot be turned back into `0xff`, + // so the builder must carry the typed text rather than fall back to the decimal. + applyExpression(gridDiv, '[Value] = 0xff'); + await asyncSetTimeout(0); + + const builder = await AdvancedFilterBuilderHarness.open(api); + const [condition] = await builder.conditionItems(); + expect(valuePillText(condition)).toBe('0xff'); + expect((await builder.openValueEditor(condition)).value).toBe('0xff'); + + // Applying from the builder round-trips the operand through the expression text, so the + // typed syntax must survive a re-open too - and must still filter on the parsed value. + await builder.apply(); + await asyncSetTimeout(0); + await builder.close(); + + const reopened = await AdvancedFilterBuilderHarness.open(api); + expect(valuePillText((await reopened.conditionItems())[0])).toBe('0xff'); + expect(api.getAdvancedFilterModel()).toEqual({ + filterType: 'bigint', + colId: 'value', + type: 'equals', + filter: '255', + }); + await new GridRows(api, 'typed hex still matches only 255').check(` + ROOT id:ROOT_NODE_ID + └── LEAF id:1 value:"255n" + `); + }); + + test('builder value editor opens with the decimal operand when no formatter is provided', async () => { + const api = gridsManager.createGrid('grid6', { + columnDefs: [ + { + field: 'value', + headerName: 'Value', + cellDataType: 'bigint', + filter: 'agBigIntColumnFilter', + filterParams: { allowedCharPattern: 'n0-9a-fA-FxX+\\-', bigintParser: parseBigInt }, + }, + ], + rowData: [{ value: 10n }, { value: 255n }, { value: 1000n }], + enableAdvancedFilter: true, + }); + await asyncSetTimeout(0); + api.setAdvancedFilterModel({ filterType: 'bigint', colId: 'value', type: 'equals', filter: '1000' } as any); + await asyncSetTimeout(0); + + const builder = await AdvancedFilterBuilderHarness.open(api); + const [condition] = await builder.conditionItems(); + expect(valuePillText(condition)).toBe('1000'); + expect((await builder.openValueEditor(condition)).value).toBe('1000'); + }); }); diff --git a/testing/behavioural/src/filters/filter-behaviour/bigint-filter-custom-parser.test.ts b/testing/behavioural/src/filters/filter-behaviour/bigint-filter-custom-parser.test.ts index 0b43c35071d..e5e23326ed5 100644 --- a/testing/behavioural/src/filters/filter-behaviour/bigint-filter-custom-parser.test.ts +++ b/testing/behavioural/src/filters/filter-behaviour/bigint-filter-custom-parser.test.ts @@ -55,4 +55,40 @@ describe('BigInt Filter — custom bigintParser', () => { └── LEAF id:0 val:"255n" `); }); + + test('hex from/to values are both parsed for a range condition', async () => { + const api: GridApi = await gridsManager.createGridAndWait('grid2', { + columnDefs: [ + { + field: 'val', + cellDataType: 'bigint', + filter: 'agBigIntColumnFilter', + filterParams: { + debounceMs: 0, + allowedCharPattern: '[\\dxXa-fA-F]', + bigintParser: (text: string | null) => + text == null || text.trim() === '' ? null : BigInt(text), + }, + }, + ], + rowData: [{ val: 1n }, { val: 16n }, { val: 100n }, { val: 255n }], + }); + + const filter = await ColumnFilterHarness.open(api, 'val'); + await filter.selectOperator('Between'); + await filter.setText('0x10', 0); + await filter.setText('0xFF', 1); + await asyncSetTimeout(0); + + expect(filter.getModel()).toEqual({ + filterType: 'bigint', + type: 'inRange', + filter: '16', + filterTo: '255', + }); + await new GridRows(api, 'hex range bounds filter to the row inside the range').check(` + ROOT id:ROOT_NODE_ID + └── LEAF id:2 val:"100n" + `); + }); }); diff --git a/testing/behavioural/src/test-utils/filters/advancedFilterBuilderHarness.ts b/testing/behavioural/src/test-utils/filters/advancedFilterBuilderHarness.ts index 58faedd476c..246445187dd 100644 --- a/testing/behavioural/src/test-utils/filters/advancedFilterBuilderHarness.ts +++ b/testing/behavioural/src/test-utils/filters/advancedFilterBuilderHarness.ts @@ -100,8 +100,8 @@ export class AdvancedFilterBuilderHarness { return this; } - /** Clicks the value pill on `item`, types `value` into the editor it opens, and commits (Enter). */ - public async setValue(item: HTMLElement, value: string): Promise { + /** Clicks the value pill on `item` and returns the editor input it opens. */ + public async openValueEditor(item: HTMLElement): Promise { const pill = item.querySelector(VALUE_PILL); if (!pill) { throw new Error('Value pill not found on builder item'); @@ -109,7 +109,7 @@ export class AdvancedFilterBuilderHarness { await firePointerLikeClick(pill); // The column/operator pills carry hidden rich-select inputs; the value editor is the only // visible one, and it mounts a macrotask or two after the click — poll rather than guess a delay. - const editor = await waitFor(() => { + return waitFor(() => { const input = Array.from(item.querySelectorAll('input.ag-text-field-input')).find( (candidate) => !candidate.closest('.ag-hidden') ); @@ -118,6 +118,11 @@ export class AdvancedFilterBuilderHarness { } return input; }); + } + + /** Clicks the value pill on `item`, types `value` into the editor it opens, and commits (Enter). */ + public async setValue(item: HTMLElement, value: string): Promise { + const editor = await this.openValueEditor(item); const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set!; setter.call(editor, value); editor.dispatchEvent(new Event('input', { bubbles: true })); From 9d69d4bb744be78750697c1777e0536acdee5717 Mon Sep 17 00:00:00 2001 From: David Skewis Date: Wed, 29 Jul 2026 16:25:03 +0100 Subject: [PATCH 2/6] AG-17915 increase products nav width (#14661) --- .../src/components/product-dropdown/ProductDropdown.module.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/ag-website-shared/src/components/product-dropdown/ProductDropdown.module.scss b/external/ag-website-shared/src/components/product-dropdown/ProductDropdown.module.scss index 0dc3f7bf443..d5c3a576d72 100644 --- a/external/ag-website-shared/src/components/product-dropdown/ProductDropdown.module.scss +++ b/external/ag-website-shared/src/components/product-dropdown/ProductDropdown.module.scss @@ -130,7 +130,7 @@ left: 0; // Fixed width gives each product row enough room for its title and // description without truncation. - width: 720px; + width: 800px; display: grid; grid-template-columns: repeat(2, minmax(0, 1fr)); } From 698cab1456133f117c36d420d6309dc12ecb94e1 Mon Sep 17 00:00:00 2001 From: Tak Tran Date: Wed, 29 Jul 2026 16:25:14 +0100 Subject: [PATCH 3/6] AG-17996 - Consent field follow-ups: error clearing, Studio consent, a11y and sandbox IDs (#14660) * AG-17996 - clear contact form errors as soon as a field is corrected QA TC1: tabbing past the unchecked data processing consent checkbox showed the required-consent error, and ticking the box left the error on screen until focus moved away again. react-hook-form's 'onBlur' mode validates on blur but does not re-validate on change before the first submit, so the error state went stale. 'onTouched' keeps the same first-validation-on-blur behaviour and adds re-validation on change, so the message clears immediately. This applies to every field on the form, not just the checkbox - correcting an invalid email now clears its error without needing to blur first. * AG-17996 - Update default values for consent fields * AG-17996 - address review findings on the consent field PRs Studio trial form (P1): the privacy notice was removed without the form gaining the consent controls that justified removing it elsewhere, leaving it collecting name and email with no disclosure at all. It now gets the same treatment as the other trial form - required data processing consent, optional marketing consent, and a France/Italy declaration that reveals optional email tracking consent - and sends all four in its JSON payload. Consent errors are now exposed to assistive technology (P2): the checkbox gets aria-invalid and aria-describedby pointing at the message, which carries a matching id and role=alert. Previously the error was an unassociated paragraph, so screen reader users had no way to learn why submission failed. Also fixes two pre-existing issues the review picked up (P2): - submitUrl was missing from the submit callback dependencies, so a changed prop would have kept posting to the previous endpoint - both trial forms were typed FunctionComponent rather than FunctionComponent, so the exported type was FunctionComponent<{}> and consumers could not type-check the submitUrl prop --- .../consent-fields/ConsentCheckbox.tsx | 18 +- .../components/contact-form/ContactForm.tsx | 5 +- .../trial-licence-form/TrialLicenceForm.tsx | 3 +- .../TrialLicenceFormStudio.tsx | 160 +++++++++++++++++- external/ag-website-shared/src/constants.ts | 8 +- 5 files changed, 180 insertions(+), 14 deletions(-) diff --git a/external/ag-website-shared/src/components/consent-fields/ConsentCheckbox.tsx b/external/ag-website-shared/src/components/consent-fields/ConsentCheckbox.tsx index d37868a1144..0a41bf568a7 100644 --- a/external/ag-website-shared/src/components/consent-fields/ConsentCheckbox.tsx +++ b/external/ag-website-shared/src/components/consent-fields/ConsentCheckbox.tsx @@ -14,13 +14,27 @@ interface Props { } export const ConsentCheckbox: FunctionComponent = ({ id, label, error, nested, inputProps }: Props) => { + const errorId = `${id}-error`; + return (
-
{error &&

{error}

}
+
+ {error && ( + + )} +
); }; diff --git a/external/ag-website-shared/src/components/contact-form/ContactForm.tsx b/external/ag-website-shared/src/components/contact-form/ContactForm.tsx index f23f810d5e2..bf23f098b9e 100644 --- a/external/ag-website-shared/src/components/contact-form/ContactForm.tsx +++ b/external/ag-website-shared/src/components/contact-form/ContactForm.tsx @@ -99,7 +99,10 @@ export const ContactForm: FunctionComponent = ({ setValue, formState: { errors }, } = useForm({ - mode: 'onBlur', + // Validate on first blur, then re-validate on every change, so a message + // clears the moment the field is corrected. Plain 'onBlur' only re-validates + // on the next blur, which leaves a stale error next to a ticked checkbox. + mode: 'onTouched', }); // Email tracking consent only applies to France and Italy, so hiding it must also diff --git a/external/ag-website-shared/src/components/trial-licence-form/TrialLicenceForm.tsx b/external/ag-website-shared/src/components/trial-licence-form/TrialLicenceForm.tsx index f2fc8d4eab3..f8288076478 100644 --- a/external/ag-website-shared/src/components/trial-licence-form/TrialLicenceForm.tsx +++ b/external/ag-website-shared/src/components/trial-licence-form/TrialLicenceForm.tsx @@ -254,6 +254,7 @@ function useTrialForm({ submitUrl }: Props) { } }, [ + submitUrl, validatedEmailError, validatedFirstNameError, validatedLastNameError, @@ -292,7 +293,7 @@ function useTrialForm({ submitUrl }: Props) { }; } -export const TrialLicenceForm: FunctionComponent = ({ submitUrl }: Props) => { +export const TrialLicenceForm: FunctionComponent = ({ submitUrl }: Props) => { const { formState, formError, diff --git a/external/ag-website-shared/src/components/trial-licence-form/TrialLicenceFormStudio.tsx b/external/ag-website-shared/src/components/trial-licence-form/TrialLicenceFormStudio.tsx index d76f77d2e1e..81aff8990d0 100644 --- a/external/ag-website-shared/src/components/trial-licence-form/TrialLicenceFormStudio.tsx +++ b/external/ag-website-shared/src/components/trial-licence-form/TrialLicenceFormStudio.tsx @@ -1,3 +1,8 @@ +import { ConsentCheckbox } from '@ag-website-shared/components/consent-fields/ConsentCheckbox'; +import { + CONSENT_LABELS, + DATA_PROCESSING_CONSENT_REQUIRED, +} from '@ag-website-shared/components/consent-fields/consentMessages'; import { Icon } from '@ag-website-shared/components/icon/Icon'; import { TRIAL_LICENCE_FORM_URL, ZI_FORM_ID } from '@constants'; import { trackTrialLicenseFormError, trackTrialLicenseFormSuccess } from '@utils/analytics'; @@ -90,22 +95,55 @@ function useRequiredValidation(initialValue: string = '') { }; } +function useCheckbox(initialValue: boolean = false) { + const [checked, setChecked] = useState(initialValue); + + const handleCheckedChange: ChangeEventHandler = useCallback((e) => { + setChecked(e.target.checked); + }, []); + + return { + checked, + setChecked, + handleCheckedChange, + }; +} + async function submitTrialLicenceFormData({ submitUrl = TRIAL_LICENCE_FORM_URL, firstName, lastName, email, company, + dataProcessingConsent, + marketingEmailConsent, + emailTrackingConsent, + franceOrItaly, }: { submitUrl?: string; firstName: string; lastName: string; email: string; company: string; + dataProcessingConsent: boolean; + marketingEmailConsent: boolean; + emailTrackingConsent: boolean; + franceOrItaly: boolean; }) { const response = await fetch(submitUrl, { method: 'POST', - body: JSON.stringify({ data: { firstName, lastName, email, company } }), + body: JSON.stringify({ + data: { + firstName, + lastName, + email, + company, + dataProcessingConsent, + marketingEmailConsent, + emailTrackingConsent, + franceOrItaly, + }, + }), headers: { 'Content-Type': 'application/json', }, @@ -136,12 +174,35 @@ function useTrialForm({ submitUrl }: Props) { } = useRequiredValidation(); const lastNameError = wasValidated && validatedLastNameError ? validatedLastNameError : ''; + const { checked: dataProcessingConsent, handleCheckedChange: handleDataProcessingConsentChange } = useCheckbox(); + const dataProcessingConsentError = wasValidated && !dataProcessingConsent ? DATA_PROCESSING_CONSENT_REQUIRED : ''; + + const { checked: marketingEmailConsent, handleCheckedChange: handleMarketingEmailConsentChange } = useCheckbox(); + + const { + checked: emailTrackingConsent, + setChecked: setEmailTrackingConsent, + handleCheckedChange: handleEmailTrackingConsentChange, + } = useCheckbox(); + + const { checked: isFranceOrItaly, setChecked: setIsFranceOrItaly } = useCheckbox(); + + // Email tracking consent only applies to France and Italy, so hiding it must also + // withdraw it — never submit a consent the visitor can no longer see + const handleFranceOrItalyChange: ChangeEventHandler = useCallback((e) => { + const { checked } = e.target; + setIsFranceOrItaly(checked); + if (!checked) { + setEmailTrackingConsent(false); + } + }, []); + const handleFormSubmit: FormEventHandler = useCallback( async (e) => { e.preventDefault(); setWasValidated(true); - if (validatedEmailError || validatedFirstNameError || validatedLastNameError) { + if (validatedEmailError || validatedFirstNameError || validatedLastNameError || !dataProcessingConsent) { setFormState('error'); return; } @@ -153,7 +214,17 @@ function useTrialForm({ submitUrl }: Props) { try { const company = (document.getElementById('company') as HTMLInputElement)?.value || ''; - const response = await submitTrialLicenceFormData({ submitUrl, firstName, lastName, email, company }); + const response = await submitTrialLicenceFormData({ + submitUrl, + firstName, + lastName, + email, + company, + dataProcessingConsent, + marketingEmailConsent, + emailTrackingConsent, + franceOrItaly: isFranceOrItaly, + }); if (response.error) { setFormState('error'); @@ -182,7 +253,19 @@ function useTrialForm({ submitUrl }: Props) { setFormState('error'); } }, - [validatedEmailError, validatedFirstNameError, validatedLastNameError, firstName, lastName, email] + [ + submitUrl, + validatedEmailError, + validatedFirstNameError, + validatedLastNameError, + firstName, + lastName, + email, + dataProcessingConsent, + marketingEmailConsent, + emailTrackingConsent, + isFranceOrItaly, + ] ); return { @@ -197,11 +280,20 @@ function useTrialForm({ submitUrl }: Props) { lastName, lastNameError, handleLastNameChange, + dataProcessingConsent, + dataProcessingConsentError, + handleDataProcessingConsentChange, + marketingEmailConsent, + handleMarketingEmailConsentChange, + emailTrackingConsent, + handleEmailTrackingConsentChange, + isFranceOrItaly, + handleFranceOrItalyChange, handleFormSubmit, }; } -export const TrialLicenceFormStudio: FunctionComponent = ({ submitUrl }: Props) => { +export const TrialLicenceFormStudio: FunctionComponent = ({ submitUrl }: Props) => { const { formState, formError, @@ -214,9 +306,18 @@ export const TrialLicenceFormStudio: FunctionComponent = ({ submitUrl }: Props) lastName, lastNameError, handleLastNameChange, + dataProcessingConsent, + dataProcessingConsentError, + handleDataProcessingConsentChange, + marketingEmailConsent, + handleMarketingEmailConsentChange, + emailTrackingConsent, + handleEmailTrackingConsentChange, + isFranceOrItaly, + handleFranceOrItalyChange, handleFormSubmit, } = useTrialForm({ submitUrl }); - const hasFormError = Boolean(emailError || firstNameError || lastNameError); + const hasFormError = Boolean(emailError || firstNameError || lastNameError || dataProcessingConsentError); return (
@@ -271,6 +372,53 @@ export const TrialLicenceFormStudio: FunctionComponent = ({ submitUrl }: Props)

{emailError ? emailError : 'Email required'}

+
+ + + + + + + {isFranceOrItaly && ( + + )} +
+