Skip to content

Commit

Permalink
fix(core): add console error if any of column def id includes dot (#198)
Browse files Browse the repository at this point in the history
- all Editors/Filters are using the column definition id to add a css className for properly identifying the this editor/filter with a unique className, however if the columnId includes a dot then that becomes 2 class names in css and will cause issues, so it's better to advise the user from the start that his column definitions has some invalid Id(s).
  • Loading branch information
ghiscoding authored Dec 14, 2020
1 parent 6a7d980 commit 6ee40af
Show file tree
Hide file tree
Showing 23 changed files with 54 additions and 42 deletions.
4 changes: 2 additions & 2 deletions packages/common/src/editors/checkboxEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ export class CheckboxEditor implements Editor {
}

init(): void {
const columnId = this.columnDef && this.columnDef.id;
const title = this.columnEditor && this.columnEditor.title || '';
const columnId = this.columnDef?.id ?? '';
const title = this.columnEditor?.title ?? '';
const compositeEditorOptions = this.args.compositeEditorOptions;

this._checkboxContainerElm = document.createElement('div');
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/editors/dateEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ export class DateEditor implements Editor {
init(): void {
if (this.args && this.columnDef) {
const compositeEditorOptions = this.args.compositeEditorOptions;
const columnId = this.columnDef && this.columnDef.id;
const placeholder = this.columnEditor && this.columnEditor.placeholder || '';
const columnId = this.columnDef?.id ?? '';
const placeholder = this.columnEditor?.placeholder ?? '';
const title = this.columnEditor && this.columnEditor.title || '';
const gridOptions = (this.args.grid.getOptions() || {}) as GridOption;
this.defaultDate = (this.args.item) ? this.args.item[this.columnDef.field] : null;
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/editors/dualInputEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export class DualInputEditor implements Editor {

createInput(position: 'leftInput' | 'rightInput'): HTMLInputElement {
const editorSideParams = this.editorParams[position];
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
const idPropName = this.gridOptions.datasetIdPropertyName || 'id';
const itemId = this.args?.item[idPropName] || 0;

Expand Down
6 changes: 3 additions & 3 deletions packages/common/src/editors/floatEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ export class FloatEditor implements Editor {

init() {
if (this.columnDef && this.columnEditor && this.args) {
const columnId = this.columnDef.id;
const placeholder = this.columnEditor.placeholder || '';
const title = this.columnEditor.title || '';
const columnId = this.columnDef?.id ?? '';
const placeholder = this.columnEditor?.placeholder ?? '';
const title = this.columnEditor?.title ?? '';
const inputStep = (this.columnEditor.valueStep !== undefined) ? this.columnEditor.valueStep : this.getInputDecimalSteps();
const compositeEditorOptions = this.args.compositeEditorOptions;

Expand Down
6 changes: 3 additions & 3 deletions packages/common/src/editors/integerEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ export class IntegerEditor implements Editor {

init() {
if (this.columnDef && this.columnEditor) {
const columnId = this.columnDef.id;
const placeholder = this.columnEditor.placeholder || '';
const title = this.columnEditor.title || '';
const columnId = this.columnDef?.id ?? '';
const placeholder = this.columnEditor?.placeholder ?? '';
const title = this.columnEditor?.title ?? '';
const inputStep = (this.columnEditor.valueStep !== undefined) ? this.columnEditor.valueStep : '1';
const compositeEditorOptions = this.args.compositeEditorOptions;

Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/editors/longTextEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export class LongTextEditor implements Editor {
}

const compositeEditorOptions = this.args.compositeEditorOptions;
const columnId = this.columnDef?.id;
const columnId = this.columnDef?.id ?? '';
const placeholder = this.columnEditor?.placeholder ?? '';
const title = this.columnEditor?.title ?? '';
const maxLength = this.columnEditor?.maxLength;
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/editors/selectEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export class SelectEditor implements Editor {
this._locales = this.gridOptions.locales || Constants.locales;

// provide the name attribute to the DOM element which will be needed to auto-adjust drop position (dropup / dropdown)
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
this.elementName = `editor-${columnId}`;
const compositeEditorOptions = this.args.compositeEditorOptions;

Expand Down
6 changes: 3 additions & 3 deletions packages/common/src/editors/sliderEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ export class SliderEditor implements Editor {
* Create the HTML template as a string
*/
private buildTemplateHtmlString() {
const fieldId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
const title = this.columnEditor && this.columnEditor.title || '';
const minValue = this.columnEditor.hasOwnProperty('minValue') ? this.columnEditor.minValue : DEFAULT_MIN_VALUE;
const maxValue = this.columnEditor.hasOwnProperty('maxValue') ? this.columnEditor.maxValue : DEFAULT_MAX_VALUE;
Expand All @@ -286,7 +286,7 @@ export class SliderEditor implements Editor {
<input type="range" name="${this._elementRangeInputId}" title="${title}"
defaultValue="${defaultValue}" value="${defaultValue}"
min="${minValue}" max="${maxValue}" step="${step}"
class="form-control slider-editor-input editor-${fieldId} range ${this._elementRangeInputId}" />
class="form-control slider-editor-input editor-${columnId} range ${this._elementRangeInputId}" />
</div>`;
}

Expand All @@ -295,7 +295,7 @@ export class SliderEditor implements Editor {
<input type="range" name="${this._elementRangeInputId}" title="${title}"
defaultValue="${defaultValue}" value="${defaultValue}"
min="${minValue}" max="${maxValue}" step="${step}"
class="form-control slider-editor-input editor-${fieldId} range ${this._elementRangeInputId}" />
class="form-control slider-editor-input editor-${columnId} range ${this._elementRangeInputId}" />
<div class="input-group-addon input-group-append slider-value"><span class="input-group-text ${this._elementRangeOutputId}">${defaultValue}</span></div>
</div>`;
}
Expand Down
6 changes: 3 additions & 3 deletions packages/common/src/editors/textEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ export class TextEditor implements Editor {
}

init() {
const columnId = this.columnDef && this.columnDef.id;
const placeholder = this.columnEditor && this.columnEditor.placeholder || '';
const title = this.columnEditor && this.columnEditor.title || '';
const columnId = this.columnDef?.id ?? '';
const placeholder = this.columnEditor?.placeholder ?? '';
const title = this.columnEditor?.title ?? '';
const compositeEditorOptions = this.args.compositeEditorOptions;

this._input = document.createElement('input') as HTMLInputElement;
Expand Down
6 changes: 3 additions & 3 deletions packages/common/src/filters/autoCompleteFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,9 @@ export class AutoCompleteFilter implements Filter {
* Create the HTML template as a string
*/
protected buildTemplateHtmlString() {
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
let placeholder = (this.gridOptions) ? (this.gridOptions.defaultFilterPlaceholder || '') : '';
if (this.columnFilter && this.columnFilter.placeholder) {
if (this.columnFilter?.placeholder) {
placeholder = this.columnFilter.placeholder;
}
return `<input type="text" role="presentation" autocomplete="off" class="form-control search-filter filter-${columnId}" placeholder="${placeholder}">`;
Expand All @@ -292,7 +292,7 @@ export class AutoCompleteFilter implements Filter {
*/
protected createDomElement(filterTemplate: string, collection: any[], searchTerm?: SearchTerm) {
this._collection = collection;
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
const $headerElm = this.grid.getHeaderRowColumn(columnId);
$($headerElm).empty();

Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/filters/compoundDateFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ export class CompoundDateFilter implements Filter {
* Create the DOM element
*/
private createDomElement(searchTerm?: SearchTerm) {
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
const $headerElm = this.grid.getHeaderRowColumn(columnId);
$($headerElm).empty();

Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/filters/compoundInputFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export class CompoundInputFilter implements Filter {
// ------------------

private buildInputHtmlString() {
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
let placeholder = (this.gridOptions) ? (this.gridOptions.defaultFilterPlaceholder || '') : '';
if (this.columnFilter && this.columnFilter.placeholder) {
placeholder = this.columnFilter.placeholder;
Expand Down Expand Up @@ -195,7 +195,7 @@ export class CompoundInputFilter implements Filter {
* Create the DOM element
*/
private createDomElement(searchTerm?: SearchTerm) {
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
const $headerElm = this.grid.getHeaderRowColumn(columnId);
$($headerElm).empty();

Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/filters/compoundSliderFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ export class CompoundSliderFilter implements Filter {
* Create the DOM element
*/
private createDomElement(searchTerm?: SearchTerm) {
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
const minValue = (this.filterProperties.hasOwnProperty('minValue') && this.filterProperties.minValue) ? this.filterProperties.minValue : DEFAULT_MIN_VALUE;
const startValue = +(this.filterParams.hasOwnProperty('sliderStartValue') ? this.filterParams.sliderStartValue : minValue);
const $headerElm = this.grid.getHeaderRowColumn(this.columnDef.id);
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/filters/dateRangeFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ export class DateRangeFilter implements Filter {
* @params searchTerms
*/
private createDomElement(searchTerms?: SearchTerm[]) {
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
const $headerElm = this.grid.getHeaderRowColumn(columnId);
$($headerElm).empty();

Expand Down
10 changes: 5 additions & 5 deletions packages/common/src/filters/inputFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,28 +125,28 @@ export class InputFilter implements Filter {
* Create the HTML template as a string
*/
protected buildTemplateHtmlString() {
const fieldId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
let placeholder = (this.gridOptions) ? (this.gridOptions.defaultFilterPlaceholder || '') : '';
if (this.columnFilter && this.columnFilter.placeholder) {
placeholder = this.columnFilter.placeholder;
}
return `<input type="${this._inputType || 'text'}" role="presentation" autocomplete="off" class="form-control search-filter filter-${fieldId}" placeholder="${placeholder}"><span></span>`;
return `<input type="${this._inputType || 'text'}" role="presentation" autocomplete="off" class="form-control search-filter filter-${columnId}" placeholder="${placeholder}"><span></span>`;
}

/**
* From the html template string, create a DOM element
* @param filterTemplate
*/
protected createDomElement(filterTemplate: string, searchTerm?: SearchTerm) {
const fieldId = this.columnDef && this.columnDef.id;
const $headerElm = this.grid.getHeaderRowColumn(fieldId);
const columnId = this.columnDef?.id ?? '';
const $headerElm = this.grid.getHeaderRowColumn(columnId);
$($headerElm).empty();

// create the DOM element & add an ID and filter class
const $filterElm = $(filterTemplate);

$filterElm.val(searchTerm as string);
$filterElm.data('columnId', fieldId);
$filterElm.data('columnId', columnId);

// if there's a search term, we will add the "filled" class for styling purposes
if (searchTerm) {
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/filters/inputMaskFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ export class InputMaskFilter extends InputFilter {
this.searchTerms = (args.hasOwnProperty('searchTerms') ? args.searchTerms : []) || [];

// get input mask from params (can be in columnDef or columnFilter params)
if (this.columnDef && this.columnDef.params && this.columnDef.params.mask) {
if (this.columnDef?.params?.mask) {
this._inputMask = this.columnDef.params.mask;
} else if (this.columnFilter && this.columnFilter.params && this.columnFilter.params.mask) {
} else if (this.columnFilter?.params?.mask) {
this._inputMask = this.columnFilter.params.mask;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/filters/nativeSelectFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export class NativeSelectFilter implements Filter {
throw new Error('The "collection" passed to the Native Select Filter is not a valid array.');
}

const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
const labelName = (this.columnFilter.customStructure) ? this.columnFilter.customStructure.label : 'label';
const valueName = (this.columnFilter.customStructure) ? this.columnFilter.customStructure.value : 'value';
const isEnabledTranslate = (this.columnFilter.enableTranslateLabel) ? this.columnFilter.enableTranslateLabel : false;
Expand Down Expand Up @@ -172,7 +172,7 @@ export class NativeSelectFilter implements Filter {
* @param filterTemplate
*/
private createDomElement(filterTemplate: string, searchTerm?: SearchTerm) {
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
const $headerElm = this.grid.getHeaderRowColumn(columnId);
$($headerElm).empty();

Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/filters/selectFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ export class SelectFilter implements Filter {
*/
protected buildTemplateHtmlString(optionCollection: any[], searchTerms: SearchTerm[]): string {
let options = '';
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
const separatorBetweenLabels = this.collectionOptions && this.collectionOptions.separatorBetweenTextLabels || '';
const isTranslateEnabled = this.gridOptions && this.gridOptions.enableTranslate;
const isRenderHtmlEnabled = this.columnFilter && this.columnFilter.enableRenderHtml || false;
Expand Down Expand Up @@ -393,7 +393,7 @@ export class SelectFilter implements Filter {
* @param filterTemplate
*/
protected createDomElement(filterTemplate: string) {
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';

// provide the name attribute to the DOM element which will be needed to auto-adjust drop position (dropup / dropdown)
this.elementName = `filter-${columnId}`;
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/filters/sliderFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export class SliderFilter implements Filter {
* Create the HTML template as a string
*/
private buildTemplateHtmlString() {
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
const minValue = this.filterProperties.hasOwnProperty('minValue') ? this.filterProperties.minValue : DEFAULT_MIN_VALUE;
const maxValue = this.filterProperties.hasOwnProperty('maxValue') ? this.filterProperties.maxValue : DEFAULT_MAX_VALUE;
const defaultValue = this.filterParams.hasOwnProperty('sliderStartValue') ? this.filterParams.sliderStartValue : minValue;
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/filters/sliderRangeFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export class SliderRangeFilter implements Filter {
* @param highestValue number
*/
renderSliderValues(lowestValue: number | string, highestValue: number | string) {
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
const lowerElm = document.querySelector(`.lowest-range-${columnId}`);
const highestElm = document.querySelector(`.highest-range-${columnId}`);
if (lowerElm && lowerElm.innerHTML) {
Expand Down Expand Up @@ -180,7 +180,7 @@ export class SliderRangeFilter implements Filter {
throw new Error(`[Slickgrid-Universal] You cannot override the "change" and/or the "slide" callback methods
since they are used in SliderRange Filter itself, however any other methods can be used for example the "create", "start", "stop" methods.`);
}
const columnId = this.columnDef && this.columnDef.id;
const columnId = this.columnDef?.id ?? '';
const $headerElm = this.grid.getHeaderRowColumn(columnId);
const minValue = this.filterProperties.hasOwnProperty('minValue') ? this.filterProperties.minValue : DEFAULT_MIN_VALUE;
const maxValue = this.filterProperties.hasOwnProperty('maxValue') ? this.filterProperties.maxValue : DEFAULT_MAX_VALUE;
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -562,14 +562,22 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
});

describe('with editors', () => {
it('should display a console error when any of the column definition ids include a dot notation', () => {
const consoleSpy = jest.spyOn(global.console, 'error').mockReturnValue();
const mockColDefs = [{ id: 'user.gender', field: 'user.gender', editor: { model: Editors.text, collection: ['male', 'female'] } }] as Column[];

component.columnDefinitions = mockColDefs;

expect(consoleSpy).toHaveBeenCalledWith('[Slickgrid-Universal] Make sure that none of your Column Definition "id" property includes a dot in its name because that will cause some problems with the Editors. For example if your column definition "field" property is "user.firstName" then use "firstName" as the column "id".');
});

it('should be able to load async editors with a regular Promise', (done) => {
const mockCollection = ['male', 'female'];
const promise = new Promise(resolve => resolve(mockCollection));
const mockColDefs = [{ id: 'gender', field: 'gender', editor: { model: Editors.text, collectionAsync: promise } }] as Column[];
const getColSpy = jest.spyOn(mockGrid, 'getColumns').mockReturnValue(mockColDefs);

component.columnDefinitions = mockColDefs;
// component.initialization(divContainer, slickEventHandler);

setTimeout(() => {
expect(getColSpy).toHaveBeenCalled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1265,6 +1265,10 @@ export class SlickVanillaGridBundle {
private swapInternalEditorToSlickGridFactoryEditor(columnDefinitions: Column[]) {
const columns = Array.isArray(columnDefinitions) ? columnDefinitions : [];

if (columns.some(col => `${col.id}`.includes('.'))) {
console.error('[Slickgrid-Universal] Make sure that none of your Column Definition "id" property includes a dot in its name because that will cause some problems with the Editors. For example if your column definition "field" property is "user.firstName" then use "firstName" as the column "id".');
}

return columns.map((column: Column) => {
// on every Editor that have a "collectionAsync", resolve the data and assign it to the "collection" property
if (column.editor?.collectionAsync) {
Expand Down

0 comments on commit 6ee40af

Please sign in to comment.