Skip to content
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.

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

fix: regression, Row Detail no longer displayed after CSP safe code #1259

Merged
merged 2 commits into from
Dec 9, 2023
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
7 changes: 6 additions & 1 deletion packages/common/src/core/slickGrid.ts
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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