Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,70 @@ describe.each([false, true])('token editor, expandToViewport=%s', expandToViewpo
expect(editor.header.getElement()).toHaveTextContent(i18nStrings.editTokenHeader!);
});

describe('changing token property to another property sets correct default value type', () => {
function changeTokenProperty(propertyKey: string) {
const propertyFilter = createWrapper().findPropertyFilter()!;
const tokens = propertyFilter.findTokens();

tokens[0].findLabel().click();
const dropdown = propertyFilter.findTokens()[0].findEditorDropdown({ expandToViewport })!;
const select = dropdown.findForm().findSelect()!;
select.openDropdown();
select.selectOptionByValue(propertyKey);
dropdown.findSubmitButton().click();
}

const baseProps: Partial<PropertyFilterProps> = {
filteringProperties: [
{ key: 'string', propertyLabel: 'string', operators: ['=', '!='], groupValuesLabel: '' },
{ key: 'other-string', propertyLabel: 'string-other', operators: ['=', '!='], groupValuesLabel: '' },
{ key: 'enum', propertyLabel: 'enum', operators: [{ operator: '=', tokenType: 'enum' }], groupValuesLabel: '' },
{
key: 'date',
propertyLabel: 'date',
operators: [{ operator: '=', form: () => <div /> }],
groupValuesLabel: '',
},
],
query: { tokens: [{ propertyKey: 'string', value: 'value', operator: '=' }], operation: 'and' },
expandToViewport,
};

test('string property initializes to string', () => {
const onChange = jest.fn();
renderComponent({ ...baseProps, onChange });

changeTokenProperty('other-string');
expect(onChange).toHaveBeenCalledWith(
expect.objectContaining({
detail: { tokens: [{ propertyKey: 'other-string', operator: '=', value: '' }], operation: 'and' },
})
);
});

test('enum property initializes to empty array', () => {
const onChange = jest.fn();
renderComponent({ ...baseProps, onChange });
changeTokenProperty('enum');
expect(onChange).toHaveBeenCalledWith(
expect.objectContaining({
detail: { tokens: [{ propertyKey: 'enum', operator: '=', value: [] }], operation: 'and' },
})
);
});

test('custom property initializes to null', () => {
const onChange = jest.fn();
renderComponent({ ...baseProps, onChange });
changeTokenProperty('date');
expect(onChange).toHaveBeenCalledWith(
expect.objectContaining({
detail: { tokens: [{ propertyKey: 'date', operator: '=', value: null }], operation: 'and' },
})
);
});
});

describe('form controls content', () => {
test('default', () => {
renderComponent({
Expand Down Expand Up @@ -501,7 +565,7 @@ describe('token editor with groups', () => {
expect.objectContaining({
detail: {
operation: 'and',
tokenGroups: [{ propertyKey: 'other-string', operator: '=', value: null }],
tokenGroups: [{ propertyKey: 'other-string', operator: '=', value: '' }],
Copy link
Member

@pan-kot pan-kot Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the issue for string properties, but breaks enum and custom properties, see: #4041

Copy link
Member Author

@jperals jperals Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Changed the logic based on the expected behavior of those tests (empty array for enum and null for custom properties). Also refactored into one test for each.

I rely on the presence of a form property in order to infer a custom property (see token-editor.tsx below? Is there a better way? Nor the internal operator, which is a string, nor the matchedProperty, which is of type InternalFilteringProperty, have enough information about this, and the PropertyFilterTokenType type returned by property.getTokenType only knows about value or enum.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, relying on the form sounds good 👍

tokens: [],
},
})
Expand Down
4 changes: 3 additions & 1 deletion src/property-filter/token-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ export function TokenEditor({
? temporaryToken.operator
: allowedOperators[0];
const matchedProperty = filteringProperties.find(property => property.propertyKey === newPropertyKey) ?? null;
setTemporaryToken({ ...temporaryToken, property: matchedProperty, operator, value: null });
const isCustomType = !!matchedProperty?.getValueFormRenderer(operator);
const value = isCustomType || matchedProperty?.getTokenType(operator) === 'enum' ? null : '';
setTemporaryToken({ ...temporaryToken, property: matchedProperty, operator, value });
};

const operator = temporaryToken.operator;
Expand Down
Loading