Skip to content

Commit

Permalink
fix(module:message): clean up DOM after usage (#7965)
Browse files Browse the repository at this point in the history
Call `overlayRef.dispose()` when all instances of messages have
disappeared

Resolves #7772
  • Loading branch information
swseverance committed Sep 18, 2023
1 parent 7470ed6 commit 71ead99
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 34 deletions.
6 changes: 6 additions & 0 deletions components/core/services/singleton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ export class NzSingletonService {
}
}

unregisterSingletonWithKey(key: string): void {
if (this.singletonRegistry.has(key)) {
this.singletonRegistry.delete(key);
}
}

getSingletonWithKey<T>(key: string): T | null {
return this.singletonRegistry.has(key) ? (this.singletonRegistry.get(key)!.target as T) : null;
}
Expand Down
19 changes: 19 additions & 0 deletions components/message/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ export abstract class NzMNService {
if (!containerInstance) {
this.container = containerInstance = componentRef.instance;
this.nzSingletonService.registerSingletonWithKey(this.componentPrefix, containerInstance);
this.container.afterAllInstancesRemoved.subscribe(() => {
this.container = undefined;
this.nzSingletonService.unregisterSingletonWithKey(this.componentPrefix);
overlayRef.dispose();
});
}

return containerInstance as T;
Expand All @@ -71,6 +76,10 @@ export abstract class NzMNContainerComponent implements OnInit, OnDestroy {
config?: Required<MessageConfig>;
instances: Array<Required<NzMessageData>> = [];

private readonly _afterAllInstancesRemoved = new Subject<void>();

readonly afterAllInstancesRemoved = this._afterAllInstancesRemoved.asObservable();

protected readonly destroy$ = new Subject<void>();

constructor(protected cdr: ChangeDetectorRef, protected nzConfigService: NzConfigService) {
Expand Down Expand Up @@ -110,13 +119,18 @@ export abstract class NzMNContainerComponent implements OnInit, OnDestroy {
this.onRemove(instance, userAction);
this.readyInstances();
});

if (!this.instances.length) {
this.onAllInstancesRemoved();
}
}

removeAll(): void {
this.instances.forEach(i => this.onRemove(i, false));
this.instances = [];

this.readyInstances();
this.onAllInstancesRemoved();
}

protected onCreate(instance: NzMessageData): Required<NzMessageData> {
Expand All @@ -130,6 +144,11 @@ export abstract class NzMNContainerComponent implements OnInit, OnDestroy {
instance.onClose.complete();
}

private onAllInstancesRemoved(): void {
this._afterAllInstancesRemoved.next();
this._afterAllInstancesRemoved.complete();
}

protected readyInstances(): void {
this.cdr.detectChanges();
}
Expand Down
39 changes: 25 additions & 14 deletions components/message/message.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ import { NzMessageService } from './message.service';
describe('message', () => {
let testBed: ComponentBed<NzTestMessageComponent>;
let messageService: NzMessageService;
let overlayContainer: OverlayContainer;
let overlayContainerElement: HTMLElement;
let fixture: ComponentFixture<NzTestMessageComponent>;
let testComponent: NzTestMessageComponent;
let nzConfigService: NzConfigService;
let configService: NzConfigService;

beforeEach(fakeAsync(() => {
testBed = createComponentBed(NzTestMessageComponent, {
Expand All @@ -38,17 +39,14 @@ describe('message', () => {
testComponent = testBed.component;
}));

beforeEach(inject([NzMessageService, OverlayContainer], (m: NzMessageService, oc: OverlayContainer) => {
messageService = m;
// need init before testing
const message = messageService.success('init');
messageService.remove(message.messageId);
// @ts-ignore
nzConfigService = messageService.container.nzConfigService;
if (!overlayContainerElement) {
overlayContainerElement = oc.getContainerElement();
beforeEach(inject(
[NzMessageService, OverlayContainer, NzConfigService],
(m: NzMessageService, oc: OverlayContainer, c: NzConfigService) => {
messageService = m;
overlayContainer = oc;
configService = c;
}
}));
));

afterEach(() => {
messageService.remove();
Expand All @@ -57,6 +55,7 @@ describe('message', () => {
it('should open a message box with success', () => {
messageService.success('SUCCESS');
fixture.detectChanges();
overlayContainerElement = overlayContainer.getContainerElement();

expect((overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement).style.zIndex).toBe('1010');
expect(overlayContainerElement.textContent).toContain('SUCCESS');
Expand All @@ -66,13 +65,15 @@ describe('message', () => {
it('should open a message box with error', () => {
messageService.error('ERROR');
fixture.detectChanges();
overlayContainerElement = overlayContainer.getContainerElement();
expect(overlayContainerElement.textContent).toContain('ERROR');
expect(overlayContainerElement.querySelector('.anticon-close-circle')).not.toBeNull();
});

it('should open a message box with warning', () => {
messageService.warning('WARNING');
fixture.detectChanges();
overlayContainerElement = overlayContainer.getContainerElement();

expect(overlayContainerElement.textContent).toContain('WARNING');
expect(overlayContainerElement.querySelector('.anticon-exclamation-circle')).not.toBeNull();
Expand All @@ -81,6 +82,7 @@ describe('message', () => {
it('should open a message box with info', () => {
messageService.info('INFO');
fixture.detectChanges();
overlayContainerElement = overlayContainer.getContainerElement();

expect(overlayContainerElement.textContent).toContain('INFO');
expect(overlayContainerElement.querySelector('.anticon-info-circle')).not.toBeNull();
Expand All @@ -89,6 +91,7 @@ describe('message', () => {
it('should open a message box with loading', () => {
messageService.loading('LOADING');
fixture.detectChanges();
overlayContainerElement = overlayContainer.getContainerElement();

expect(overlayContainerElement.textContent).toContain('LOADING');
expect(overlayContainerElement.querySelector('.anticon-loading')).not.toBeNull();
Expand All @@ -97,6 +100,7 @@ describe('message', () => {
it('should support template', fakeAsync(() => {
messageService.info(testComponent.template);
fixture.detectChanges();
overlayContainerElement = overlayContainer.getContainerElement();

expect(overlayContainerElement.textContent).toContain('Content in template');
tick(10000);
Expand All @@ -105,6 +109,7 @@ describe('message', () => {
it('should auto closed by 1s', fakeAsync(() => {
messageService.create('', 'EXISTS', { nzDuration: 1000 });
fixture.detectChanges();
overlayContainerElement = overlayContainer.getContainerElement();

expect(overlayContainerElement.textContent).toContain('EXISTS');

Expand All @@ -115,6 +120,7 @@ describe('message', () => {
it('should not destroy when hovered', fakeAsync(() => {
messageService.create('', 'EXISTS', { nzDuration: 3000 });
fixture.detectChanges();
overlayContainerElement = overlayContainer.getContainerElement();

const messageElement = overlayContainerElement.querySelector('.ant-message-notice')!;
dispatchMouseEvent(messageElement, 'mouseenter');
Expand All @@ -129,6 +135,7 @@ describe('message', () => {
it('should not destroyed automatically but manually', fakeAsync(() => {
const filledMessage = messageService.success('SUCCESS', { nzDuration: 0 });
fixture.detectChanges();
overlayContainerElement = overlayContainer.getContainerElement();

tick(50000);
expect(overlayContainerElement.textContent).toContain('SUCCESS');
Expand All @@ -145,6 +152,7 @@ describe('message', () => {
fixture.detectChanges();
tick();
fixture.detectChanges();
overlayContainerElement = overlayContainer.getContainerElement();

expect(overlayContainerElement.textContent).toContain(content);
if (id === 3) {
Expand All @@ -156,7 +164,7 @@ describe('message', () => {
messageService.remove();
fixture.detectChanges();
expect(overlayContainerElement.textContent).not.toContain('SUCCESS-3');
expect((messageService as any).container.instances.length).toBe(0); // eslint-disable-line @typescript-eslint/no-explicit-any
expect((messageService as any).container).toBeUndefined(); // eslint-disable-line @typescript-eslint/no-explicit-any
}));

it('should destroy without animation', fakeAsync(() => {
Expand All @@ -167,10 +175,11 @@ describe('message', () => {
}));

it('should reset default config from config service', fakeAsync(() => {
nzConfigService.set('message', { nzDuration: 0 });
configService.set('message', { nzDuration: 0 });
messageService.create('loading', 'EXISTS');
fixture.detectChanges();
tick(10000);
overlayContainerElement = overlayContainer.getContainerElement();
expect(overlayContainerElement.textContent).toContain('EXISTS');
}));

Expand All @@ -190,6 +199,7 @@ describe('message', () => {
messageService.create('top', 'CHANGE');
fixture.detectChanges();

overlayContainerElement = overlayContainer.getContainerElement();
const messageContainerElement = overlayContainerElement.querySelector('.ant-message') as HTMLElement;
expect(messageContainerElement.style.top).toBe('24px');

Expand All @@ -198,9 +208,10 @@ describe('message', () => {

describe('RTL', () => {
it('should apply classname', () => {
nzConfigService.set('message', { nzDirection: 'rtl' });
configService.set('message', { nzDirection: 'rtl' });
messageService.info('INFO');
fixture.detectChanges();
overlayContainerElement = overlayContainer.getContainerElement();
expect(overlayContainerElement.textContent).toContain('INFO');
expect(overlayContainerElement.querySelector('.ant-message-rtl')).not.toBeNull();
});
Expand Down
Loading

0 comments on commit 71ead99

Please sign in to comment.