Skip to content

Commit

Permalink
fix: regression, Row Detail no longer displayed after CSP safe code (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
ghiscoding committed Dec 9, 2023
1 parent 8f706fc commit a35f0a4
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 27 deletions.
7 changes: 6 additions & 1 deletion packages/common/src/core/slickGrid.ts
Expand Up @@ -77,7 +77,7 @@ import type {
SlickPlugin,
SlickGridEventData,
} from '../interfaces';
import { createDomElement, emptyElement, getOffset, getInnerSize } from '../services/domUtilities';
import { createDomElement, emptyElement, getInnerSize, getOffset, insertAfterElement } from '../services/domUtilities';

/**
* @license
Expand Down Expand Up @@ -3481,6 +3481,11 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e

divRow.appendChild(cellDiv);

// Formatter can optional add an "insertElementAfterTarget" option but it must be inserted only after the `.slick-row` div exists
if ((formatterResult as FormatterResultObject).insertElementAfterTarget) {
insertAfterElement(cellDiv, (formatterResult as FormatterResultObject).insertElementAfterTarget as HTMLElement);
}

this.rowsCache[row].cellRenderQueue.push(cell);
this.rowsCache[row].cellColSpans[cell] = colspan;
}
Expand Down
Expand Up @@ -7,6 +7,13 @@ export interface FormatterResultObject {

/** Optional tooltip text when hovering the cell div container. */
toolTip?: string;

/**
* optionally insert an HTML element after the element target
* for example we use this technique to take a div containing the row detail and insert it after the `.slick-cell`
* e.g.: <div class="slick-cell"></div><div class="row-detail">...</div>
*/
insertElementAfterTarget?: HTMLElement;
}

export interface FormatterResultWithText extends FormatterResultObject {
Expand Down
5 changes: 5 additions & 0 deletions packages/common/src/services/domUtilities.ts
Expand Up @@ -348,6 +348,11 @@ export function htmlEncodeWithPadding(inputStr: string, paddingLength: number):
return outputStr;
}

/** insert an HTML Element after a target Element in the DOM */
export function insertAfterElement(referenceNode: HTMLElement, newNode: HTMLElement) {
referenceNode.parentNode?.insertBefore(newNode, referenceNode.nextSibling);
}

/**
* Sanitize possible dirty html string (remove any potential XSS code like scripts and others), we will use 2 possible sanitizer
* 1. optional sanitizer method defined in the grid options
Expand Down
@@ -1,5 +1,5 @@
import 'jest-extended';
import { Column, GridOption, PubSubService, type SlickDataView, SlickEvent, SlickEventData, SlickGrid } from '@slickgrid-universal/common';
import { Column, GridOption, PubSubService, type SlickDataView, SlickEvent, SlickEventData, SlickGrid, FormatterResultWithHtml } from '@slickgrid-universal/common';

import { SlickRowDetailView } from './slickRowDetailView';

Expand Down Expand Up @@ -39,6 +39,7 @@ const gridStub = {
invalidateRows: jest.fn(),
registerPlugin: jest.fn(),
render: jest.fn(),
sanitizeHtmlString: (s) => s,
updateRowCount: jest.fn(),
onBeforeEditCell: new SlickEvent(),
onClick: new SlickEvent(),
Expand Down Expand Up @@ -715,7 +716,7 @@ describe('SlickRowDetailView plugin', () => {
plugin.setOptions({ collapsedClass: 'some-collapsed' });
plugin.expandableOverride(() => true);
const formattedVal = plugin.getColumnDefinition().formatter!(0, 1, '', mockColumns[0], mockItem, gridStub);
expect(formattedVal).toBe(`<div class="detailView-toggle expand some-collapsed"></div>`);
expect((formattedVal as HTMLElement).outerHTML).toBe(`<div class="detailView-toggle expand some-collapsed"></div>`);
});

it('should execute formatter and expect it to return empty string and render nothing when isPadding is True', () => {
Expand All @@ -733,7 +734,8 @@ describe('SlickRowDetailView plugin', () => {
plugin.setOptions({ expandedClass: 'some-expanded', maxRows: 2 });
plugin.expandableOverride(() => true);
const formattedVal = plugin.getColumnDefinition().formatter!(0, 1, '', mockColumns[0], mockItem, gridStub);
expect(formattedVal).toBe(`<div class="detailView-toggle collapse some-expanded"></div></div><div class="dynamic-cell-detail cellDetailView_123" style="height: 50px;top: 25px"><div class="detail-container detailViewContainer_123"><div class="innerDetailView_123">undefined</div></div>`);
expect(((formattedVal as FormatterResultWithHtml).html as HTMLElement).outerHTML).toBe(`<div class="detailView-toggle collapse some-expanded"></div>`);
expect((formattedVal as FormatterResultWithHtml).insertElementAfterTarget!.outerHTML).toBe(`<div class=\"dynamic-cell-detail cellDetailView_123\" style=\"height: 50px; top: 25px;\"><div class=\"detail-container detailViewContainer_123\"><div class=\"innerDetailView_123\">undefined</div></div></div>`);
});
});
});
46 changes: 23 additions & 23 deletions packages/row-detail-view-plugin/src/slickRowDetailView.ts
Expand Up @@ -3,7 +3,6 @@ import type {
DOMMouseOrTouchEvent,
ExternalResource,
FormatterResultWithHtml,
FormatterResultWithText,
GridOption,
OnAfterRowDetailToggleArgs,
OnBeforeRowDetailToggleArgs,
Expand All @@ -16,11 +15,11 @@ import type {
RowDetailViewOption,
SlickGrid,
SlickRowDetailView as UniversalRowDetailView,
UsabilityOverrideFn,
SlickDataView,
SlickEventData,
UsabilityOverrideFn,
} from '@slickgrid-universal/common';
import { SlickEvent, SlickEventHandler, } from '@slickgrid-universal/common';
import { createDomElement, SlickEvent, SlickEventHandler, } from '@slickgrid-universal/common';
import { objectAssignAndExtend } from '@slickgrid-universal/utils';

/**
Expand Down Expand Up @@ -606,7 +605,7 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV
}

/** The Formatter of the toggling icon of the Row Detail */
protected detailSelectionFormatter(row: number, cell: number, value: any, columnDef: Column, dataContext: any, grid: SlickGrid): FormatterResultWithHtml | FormatterResultWithText | string {
protected detailSelectionFormatter(row: number, cell: number, value: any, columnDef: Column, dataContext: any, grid: SlickGrid): FormatterResultWithHtml | HTMLElement | '' {
if (!this.checkExpandableOverride(row, dataContext, grid)) {
return '';
} else {
Expand All @@ -626,9 +625,8 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV
if (this._addonOptions.collapsedClass) {
collapsedClasses += this._addonOptions.collapsedClass;
}
return `<div class="${collapsedClasses.trim()}"></div>`;
return createDomElement('div', { className: collapsedClasses.trim() });
} else {
const html: string[] = [];
const rowHeight = this.gridOptions.rowHeight || 0;
let outterHeight = (dataContext[`${this._keyPrefix}sizePadding`] || 0) * this.gridOptions.rowHeight!;

Expand All @@ -637,28 +635,30 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV
dataContext[`${this._keyPrefix}sizePadding`] = this._addonOptions.maxRows;
}

// V313HAX:
// putting in an extra closing div after the closing toggle div and ommiting a
// final closing div for the detail ctr div causes the slickgrid renderer to
// insert our detail div as a new column ;) ~since it wraps whatever we provide
// in a generic div column container. so our detail becomes a child directly of
// the row not the cell. nice =) ~no need to apply a css change to the parent
// slick-cell to escape the cell overflow clipping.

// sneaky extra </div> inserted here-----------------v
let expandedClasses = `${this._addonOptions.cssClass || ''} collapse `;
if (this._addonOptions.expandedClass) {
expandedClasses += this._addonOptions.expandedClass;
}
html.push(`<div class="${expandedClasses.trim()}"></div></div>`);
html.push(`<div class="dynamic-cell-detail cellDetailView_${dataContext[this._dataViewIdProperty]}" `); // apply custom css to detail
html.push(`style="height: ${outterHeight}px;`); // set total height of padding
html.push(`top: ${rowHeight}px">`); // shift detail below 1st row
html.push(`<div class="detail-container detailViewContainer_${dataContext[this._dataViewIdProperty]}">`); // sub ctr for custom styling
html.push(`<div class="innerDetailView_${dataContext[this._dataViewIdProperty]}">${dataContext[`${this._keyPrefix}detailContent`]}</div></div>`);
// omit a final closing detail container </div> that would come next

return html.join('');

// create the Row Detail div container that will be inserted AFTER the `.slick-cell`
const cellDetailContainerElm = createDomElement('div', {
className: `dynamic-cell-detail cellDetailView_${dataContext[this._dataViewIdProperty]}`,
style: { height: `${outterHeight}px`, top: `${rowHeight}px` }
});
const innerContainerElm = createDomElement('div', { className: `detail-container detailViewContainer_${dataContext[this._dataViewIdProperty]}` });
const innerDetailViewElm = createDomElement('div', { className: `innerDetailView_${dataContext[this._dataViewIdProperty]}` });
innerDetailViewElm.innerHTML = this._grid.sanitizeHtmlString(dataContext[`${this._keyPrefix}detailContent`]);

innerContainerElm.appendChild(innerDetailViewElm);
cellDetailContainerElm.appendChild(innerContainerElm);

const result: FormatterResultWithHtml = {
html: createDomElement('div', { className: expandedClasses }),
insertElementAfterTarget: cellDetailContainerElm,
};

return result;
}
}
return '';
Expand Down

0 comments on commit a35f0a4

Please sign in to comment.