Skip to content

Jhork/clear modifier field bug fix #168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jonnyhork merged 8 commits into from
Feb 5, 2021
Merged

Jhork/clear modifier field bug fix #168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6ca87af
remove check for selectedConditions.length
jonnyhork Feb 4, 2021
4429d5c
adjust test for custom select component
jonnyhork Feb 4, 2021
6531055
De-couple display field from condition model
jgellin-sf Feb 4, 2021
c7c143a
Explicitly clear inputs on X click
jgellin-sf Feb 4, 2021
e1a63bd
remove dead code
jonnyhork Feb 4, 2021
786f25f
track selected field to re-render if cleared
jonnyhork Feb 4, 2021
ddf9b1d
simplify syntax
jonnyhork Feb 4, 2021
cb753a2
check all values are cleared with X click
jonnyhork Feb 4, 2021
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
5 changes: 1 addition & 4 deletions packages/soql-builder-ui/src/modules/querybuilder/customSelect/customSelect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,7 @@ export default class CustomSelect extends LightningElement {
}

set selectedOptions(selectedOptions: string[]) {
this._selectedOptions = selectedOptions || [];
if (selectedOptions && selectedOptions.length) {
this._value = selectedOptions;
}
this._selectedOptions = this._value = selectedOptions || [];
}

@api
Expand Down
35 changes: 32 additions & 3 deletions ...es/soql-builder-ui/src/modules/querybuilder/whereModifierGroup/whereModifierGroup.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,36 @@ describe('WhereModifierGroup should', () => {
expect(handler).toHaveBeenCalled();
});

it.skip('updates inputs when condition model changes', () => {
it('clears all the values when the X is clicked', () => {
Copy link
Contributor Author

@jonnyhork jonnyhork Feb 4, 2021

Choose a reason for hiding this comment

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

Do you think it is worth having a similar test but when there is an error state, like in
set error class on invalid operator input

Copy link
Contributor

@dehru dehru Feb 5, 2021

Choose a reason for hiding this comment

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

Yes, i'm adding that now. It looks like the error state needs to be cleared as well in this case.

Copy link
Contributor

@dehru dehru Feb 5, 2021

Choose a reason for hiding this comment

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

That'll be in a separate PR. QAing this now.

modifierGroup.condition = {
field: { fieldName: 'foo' },
operator: '!=',
compareValue: { type: 'STRING', value: "'HELLO'" }
};
document.body.appendChild(modifierGroup);

const clearConditionBtn = modifierGroup.shadowRoot.querySelector(
'[data-el-where-delete]'
);
const {
selectFieldEl,
selectOperatorEl,
criteriaInputEl
} = getModifierElements();

expect(selectFieldEl.value[0]).toEqual('foo');
expect(selectOperatorEl.value).toEqual('NOT_EQ');
expect(criteriaInputEl.value).toEqual('HELLO');

clearConditionBtn.click();
Promise.resolve().then(() => {
expect(selectFieldEl.value).toEqual([]);
expect(selectOperatorEl.value).toEqual('EQ');
expect(criteriaInputEl.value).toEqual('');
});
});

it('updates inputs when condition model changes', () => {
modifierGroup.condition = {
field: { fieldName: 'foo' },
operator: '!=',
Expand All @@ -159,15 +188,15 @@ describe('WhereModifierGroup should', () => {
selectOperatorEl,
criteriaInputEl
} = getModifierElements();
expect(selectFieldEl.value).toEqual('foo');
expect(selectFieldEl.value[0]).toEqual('foo');
expect(selectOperatorEl.value).toEqual('NOT_EQ');
expect(criteriaInputEl.value).toEqual('HELLO');

modifierGroup.condition = {
operator: '='
};
return Promise.resolve().then(() => {
expect(selectFieldEl.value).toEqual('');
expect(selectFieldEl.value).toEqual([]);
expect(selectOperatorEl.value).toEqual('EQ');
expect(criteriaInputEl.value).toEqual('');
});
Expand Down
27 changes: 15 additions & 12 deletions packages/soql-builder-ui/src/modules/querybuilder/whereModifierGroup/whereModifierGroup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ import {
soqlStringLiteralToDisplayValue
} from '../services/soqlUtils';

const DEFAULT_FIELD_INPUT_VALUE = '';
const DEFAULT_OPERATOR_INPUT_VALUE = 'EQ';
const DEFAULT_CRITERIA_INPUT_VALUE = '';

export default class WhereModifierGroup extends LightningElement {
@api allFields: string[];
@api isLoading = false;
Expand All @@ -34,6 +38,8 @@ export default class WhereModifierGroup extends LightningElement {
}
_condition: JsonMap;
_currentOperatorValue;
@track
_currentFieldSelection;
@track _criteriaDisplayValue;
_sobjectMetadata: any;
sobjectTypeUtils: SObjectTypeUtils;
Expand All @@ -60,6 +66,8 @@ export default class WhereModifierGroup extends LightningElement {
this._condition = condition;
this._criteriaDisplayValue = '';

this._currentFieldSelection = this.getFieldName();

const matchingOption = condition
? operatorOptions.find(
(option) => option.modelValue === condition.operator
Expand Down Expand Up @@ -122,18 +130,8 @@ export default class WhereModifierGroup extends LightningElement {
}

/* --- FIELDS --- */
get hasSelectedField() {
return !!this.getFieldName();
}

get _selectedField() {
return this.getFieldName() ? [this.getFieldName()] : [];
}

get filteredFields() {
return this.allFields.filter((field) => {
return field !== this.getFieldName();
});
return this._currentFieldSelection ? [this._currentFieldSelection] : [];
}
Copy link
Contributor Author

@jonnyhork jonnyhork Feb 4, 2021

Choose a reason for hiding this comment

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

This is dead code, I forgot to remove this when we switched to the custom select


getFieldName(): string | undefined {
Expand Down Expand Up @@ -205,6 +203,11 @@ export default class WhereModifierGroup extends LightningElement {
/** end css class methods */

handleConditionRemoved(e) {
// reset inputs to defaults
this._currentFieldSelection = DEFAULT_FIELD_INPUT_VALUE;
this._currentOperatorValue = DEFAULT_OPERATOR_INPUT_VALUE;
this._criteriaDisplayValue = DEFAULT_CRITERIA_INPUT_VALUE;

e.preventDefault();
const conditionRemovedEvent = new CustomEvent('where__condition_removed', {
detail: {
Expand Down Expand Up @@ -309,7 +312,7 @@ export default class WhereModifierGroup extends LightningElement {
if (this.checkAllModifiersHaveValues()) {
this.resetErrorFlagsAndMessages();

const fieldName = this.fieldEl.value[0];
const fieldName = (this._currentFieldSelection = this.fieldEl.value[0]);
const op = (this._currentOperatorValue = this.operatorEl.value);
const opModelValue = this.toOperatorModelValue(op);

Expand Down