Skip to content

Commit 8b1437c

Browse files
authored
fix: Row Detail open/close multiple times should always re-render (#1959)
1 parent dd7938f commit 8b1437c

File tree

14 files changed

+142
-54
lines changed

14 files changed

+142
-54
lines changed

.vscode/tasks.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
{
1111
"label": "Run Angular Unit Tests (watch)",
1212
"type": "shell",
13-
"command": "pnpm angular:test:watch",
13+
"command": "pnpm angular:test",
1414
"problemMatcher": []
1515
},
1616
{
@@ -22,7 +22,7 @@
2222
{
2323
"label": "Run Vanilla Unit Tests (watch)",
2424
"type": "shell",
25-
"command": "pnpm test:watch",
25+
"command": "pnpm test",
2626
"problemMatcher": []
2727
},
2828
{

README.md

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,6 @@ You must go through Installation Steps 1-3 prior to running the Vitest unit test
163163
To run all unit tests (with Vitest), you can run one of the following commands (make sure that steps 1-3 were executed prior to running this command)
164164
```bash
165165
pnpm run test
166-
167-
# or run Vitest in watch mode
168-
pnpm run test:watch
169166
```
170167

171168
#### Cypress E2E Tests

demos/aurelia/test/cypress/e2e/example19.cy.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,4 +243,21 @@ describe('Example 19 - Row Detail View', () => {
243243

244244
cy.get('#grid19').find('.slick-cell + .dynamic-cell-detail .innerDetailView_101').should('not.exist');
245245
});
246+
247+
it('should expect the Row Detail to be re-rendered after expanding/collapsing multiple times', () => {
248+
cy.get('#grid19').find('.slick-row:nth(1) .slick-cell:nth(0)').as('toggle1');
249+
cy.get('@toggle1').click();
250+
cy.get('@toggle1').click();
251+
cy.get('@toggle1').click();
252+
253+
cy.get('#grid19').find('.slick-cell + .dynamic-cell-detail .innerDetailView_1').as('detailContainer');
254+
cy.get('@detailContainer').find('h3').contains('Task 1');
255+
256+
cy.get('@toggle1').click();
257+
cy.get('@detailContainer').should('not.exist');
258+
259+
cy.get('@toggle1').click();
260+
cy.get('#grid19').find('.slick-cell + .dynamic-cell-detail .innerDetailView_1').as('detailContainer');
261+
cy.get('@detailContainer').find('h3').contains('Task 1');
262+
});
246263
});

demos/react/test/cypress/e2e/example19.cy.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,4 +243,21 @@ describe('Example 19 - Row Detail View', () => {
243243

244244
cy.get('#grid19').find('.slick-cell + .dynamic-cell-detail .innerDetailView_101').should('not.exist');
245245
});
246+
247+
it('should expect the Row Detail to be re-rendered after expanding/collapsing multiple times', () => {
248+
cy.get('#grid19').find('.slick-row:nth(1) .slick-cell:nth(0)').as('toggle1');
249+
cy.get('@toggle1').click();
250+
cy.get('@toggle1').click();
251+
cy.get('@toggle1').click();
252+
253+
cy.get('#grid19').find('.slick-cell + .dynamic-cell-detail .innerDetailView_1').as('detailContainer');
254+
cy.get('@detailContainer').find('h3').contains('Task 1');
255+
256+
cy.get('@toggle1').click();
257+
cy.get('@detailContainer').should('not.exist');
258+
259+
cy.get('@toggle1').click();
260+
cy.get('#grid19').find('.slick-cell + .dynamic-cell-detail .innerDetailView_1').as('detailContainer');
261+
cy.get('@detailContainer').find('h3').contains('Task 1');
262+
});
246263
});

demos/vue/test/cypress/e2e/example19.cy.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,4 +243,21 @@ describe('Example 19 - Row Detail View', () => {
243243

244244
cy.get('#grid19').find('.slick-cell + .dynamic-cell-detail .innerDetailView_101').should('not.exist');
245245
});
246+
247+
it('should expect the Row Detail to be re-rendered after expanding/collapsing multiple times', () => {
248+
cy.get('#grid19').find('.slick-row:nth(1) .slick-cell:nth(0)').as('toggle1');
249+
cy.get('@toggle1').click();
250+
cy.get('@toggle1').click();
251+
cy.get('@toggle1').click();
252+
253+
cy.get('#grid19').find('.slick-cell + .dynamic-cell-detail .innerDetailView_1').as('detailContainer');
254+
cy.get('@detailContainer').find('h3').contains('Task 1');
255+
256+
cy.get('@toggle1').click();
257+
cy.get('@detailContainer').should('not.exist');
258+
259+
cy.get('@toggle1').click();
260+
cy.get('#grid19').find('.slick-cell + .dynamic-cell-detail .innerDetailView_1').as('detailContainer');
261+
cy.get('@detailContainer').find('h3').contains('Task 1');
262+
});
246263
});

frameworks/angular-slickgrid/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@
3535
"lint:fix": "eslint --fix .",
3636
"pack-tarball": "npm pack",
3737
"pack-lib": "npm pack ./dist",
38-
"test": "ng test",
39-
"test:watch": "ng test --watch",
38+
"test": "ng test --watch",
4039
"test:coverage": "vitest --no-watch --coverage"
4140
},
4241
"publishConfig": {

frameworks/angular-slickgrid/src/library/extensions/__tests__/slickRowDetailView.spec.ts

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -262,35 +262,39 @@ describe('SlickRowDetailView', () => {
262262
expect(onRowBackViewSpy).not.toHaveBeenCalled();
263263
});
264264

265-
it('should call internal event handler subscribe and expect the "onAsyncEndUpdate" option to be called when addon notify is called', () => {
266-
// const handlerSpy = vi.spyOn(plugin.eventHandler, 'subscribe');
267-
const renderSpy = vi.spyOn(plugin, 'renderViewModel');
265+
it('should call internal event handler subscribe and expect the "onAsyncEndUpdate" option to be called when addon notify is called', () =>
266+
new Promise((done: any) => {
267+
// const handlerSpy = vi.spyOn(plugin.eventHandler, 'subscribe');
268+
const renderSpy = vi.spyOn(plugin, 'renderViewModel');
268269

269-
const onAsyncRespSpy = vi.spyOn(gridOptionsMock.rowDetailView as RowDetailView, 'onAsyncResponse');
270-
const onAsyncEndSpy = vi.spyOn(gridOptionsMock.rowDetailView as RowDetailView, 'onAsyncEndUpdate');
271-
const onAfterRowSpy = vi.spyOn(gridOptionsMock.rowDetailView as RowDetailView, 'onAfterRowDetailToggle');
272-
const onBeforeRowSpy = vi.spyOn(gridOptionsMock.rowDetailView as RowDetailView, 'onBeforeRowDetailToggle');
273-
const onRowOutViewSpy = vi.spyOn(gridOptionsMock.rowDetailView as RowDetailView, 'onRowOutOfViewportRange');
274-
const onRowBackViewSpy = vi.spyOn(gridOptionsMock.rowDetailView as RowDetailView, 'onRowBackToViewportRange');
270+
const onAsyncRespSpy = vi.spyOn(gridOptionsMock.rowDetailView as RowDetailView, 'onAsyncResponse');
271+
const onAsyncEndSpy = vi.spyOn(gridOptionsMock.rowDetailView as RowDetailView, 'onAsyncEndUpdate');
272+
const onAfterRowSpy = vi.spyOn(gridOptionsMock.rowDetailView as RowDetailView, 'onAfterRowDetailToggle');
273+
const onBeforeRowSpy = vi.spyOn(gridOptionsMock.rowDetailView as RowDetailView, 'onBeforeRowDetailToggle');
274+
const onRowOutViewSpy = vi.spyOn(gridOptionsMock.rowDetailView as RowDetailView, 'onRowOutOfViewportRange');
275+
const onRowBackViewSpy = vi.spyOn(gridOptionsMock.rowDetailView as RowDetailView, 'onRowBackToViewportRange');
275276

276-
plugin.init(gridStub);
277-
plugin.onAsyncEndUpdate = new SlickEvent();
278-
plugin.register();
279-
plugin.onAsyncEndUpdate.notify({ item: columnsMock[0], grid: gridStub }, new SlickEventData(), gridStub);
277+
plugin.init(gridStub);
278+
plugin.onAsyncEndUpdate = new SlickEvent();
279+
plugin.register();
280+
plugin.onAsyncEndUpdate.notify({ item: columnsMock[0], grid: gridStub }, new SlickEventData(), gridStub);
280281

281-
// expect(handlerSpy).toHaveBeenCalledTimes(8); // there are an extra 2x on the grid itself
282-
// expect(handlerSpy).toHaveBeenCalledWith(
283-
// { notify: expect.anything(), subscribe: expect.anything(), unsubscribe: expect.anything(), },
284-
// expect.anything()
285-
// );
286-
expect(onAsyncRespSpy).not.toHaveBeenCalled();
287-
expect(onAsyncEndSpy).toHaveBeenCalledWith(expect.anything(), { item: columnsMock[0], grid: gridStub });
288-
expect(renderSpy).toHaveBeenCalledWith({ cssClass: 'red', field: 'field1', id: 'field1', width: 100 });
289-
expect(onAfterRowSpy).not.toHaveBeenCalled();
290-
expect(onBeforeRowSpy).not.toHaveBeenCalled();
291-
expect(onRowOutViewSpy).not.toHaveBeenCalled();
292-
expect(onRowBackViewSpy).not.toHaveBeenCalled();
293-
});
282+
// expect(handlerSpy).toHaveBeenCalledTimes(8); // there are an extra 2x on the grid itself
283+
// expect(handlerSpy).toHaveBeenCalledWith(
284+
// { notify: expect.anything(), subscribe: expect.anything(), unsubscribe: expect.anything(), },
285+
// expect.anything()
286+
// );
287+
setTimeout(() => {
288+
expect(onAsyncRespSpy).not.toHaveBeenCalled();
289+
expect(onAsyncEndSpy).toHaveBeenCalledWith(expect.anything(), { item: columnsMock[0], grid: gridStub });
290+
expect(renderSpy).toHaveBeenCalledWith({ cssClass: 'red', field: 'field1', id: 'field1', width: 100 });
291+
expect(onAfterRowSpy).not.toHaveBeenCalled();
292+
expect(onBeforeRowSpy).not.toHaveBeenCalled();
293+
expect(onRowOutViewSpy).not.toHaveBeenCalled();
294+
expect(onRowBackViewSpy).not.toHaveBeenCalled();
295+
done();
296+
});
297+
}));
294298

295299
it('should call internal event handler subscribe and expect the "onAfterRowDetailToggle" option to be called when addon notify is called', () => {
296300
// const handlerSpy = vi.spyOn(plugin.eventHandler, 'subscribe');

frameworks/angular-slickgrid/src/library/extensions/slickRowDetailView.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,14 @@ export class SlickRowDetailView extends UniversalSlickRowDetailView {
149149
this._preloadCompRef?.destroy();
150150

151151
// triggers after backend called "onAsyncResponse.notify()"
152-
this.renderViewModel(args?.item);
152+
// because of the preload destroy above, we need a small delay to make sure the DOM element is ready to render the Row Detail
153+
queueMicrotask(() => {
154+
this.renderViewModel(args?.item);
153155

154-
if (this.rowDetailViewOptions && typeof this.rowDetailViewOptions.onAsyncEndUpdate === 'function') {
155-
this.rowDetailViewOptions.onAsyncEndUpdate(e, args);
156-
}
156+
if (this.rowDetailViewOptions && typeof this.rowDetailViewOptions.onAsyncEndUpdate === 'function') {
157+
this.rowDetailViewOptions.onAsyncEndUpdate(e, args);
158+
}
159+
});
157160
});
158161

159162
this.eventHandler.subscribe(

frameworks/angular-slickgrid/test/cypress/e2e/example19.cy.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,4 +243,21 @@ describe('Example 19 - Row Detail View', () => {
243243

244244
cy.get('#grid19').find('.slick-cell + .dynamic-cell-detail .innerDetailView_101').should('not.exist');
245245
});
246+
247+
it('should expect the Row Detail to be re-rendered after expanding/collapsing multiple times', () => {
248+
cy.get('#grid19').find('.slick-row:nth(1) .slick-cell:nth(0)').as('toggle1');
249+
cy.get('@toggle1').click();
250+
cy.get('@toggle1').click();
251+
cy.get('@toggle1').click();
252+
253+
cy.get('#grid19').find('.slick-cell + .dynamic-cell-detail .innerDetailView_1').as('detailContainer');
254+
cy.get('@detailContainer').find('h3').contains('Task 1');
255+
256+
cy.get('@toggle1').click();
257+
cy.get('@detailContainer').should('not.exist');
258+
259+
cy.get('@toggle1').click();
260+
cy.get('#grid19').find('.slick-cell + .dynamic-cell-detail .innerDetailView_1').as('detailContainer');
261+
cy.get('@detailContainer').find('h3').contains('Task 1');
262+
});
246263
});

frameworks/aurelia-slickgrid/src/extensions/slickRowDetailView.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,14 @@ export class SlickRowDetailView extends UniversalSlickRowDetailView {
137137
this._preloadController?.dispose();
138138

139139
// triggers after backend called "onAsyncResponse.notify()"
140-
await this.renderViewModel(args?.item);
141-
142-
if (typeof this.rowDetailViewOptions?.onAsyncEndUpdate === 'function') {
143-
this.rowDetailViewOptions.onAsyncEndUpdate(event, args);
144-
}
140+
// because of the preload destroy above, we need a small delay to make sure the DOM element is ready to render the Row Detail
141+
queueMicrotask(async () => {
142+
await this.renderViewModel(args?.item);
143+
144+
if (typeof this.rowDetailViewOptions?.onAsyncEndUpdate === 'function') {
145+
this.rowDetailViewOptions.onAsyncEndUpdate(event, args);
146+
}
147+
});
145148
});
146149

147150
this._eventHandler.subscribe(this.onAfterRowDetailToggle, async (event, args) => {

0 commit comments

Comments
 (0)