Skip to content

Commit

Permalink
fix(modal): cmpt based content was not scrollable (#3286)
Browse files Browse the repository at this point in the history
In `5.0.0` we introduced the bootstrap scrollable content feature.
This behaviour appears to be broken with modal that have content generated
from a component. Overall styling is wrong, resulting in a broken modal
not scrollable.

The fix is adding some extra styling added via a CSS classname
`component-host-scrollable`, in order to force the component
host to fit inside the parent `.model-content` flex display.

A complete different, and non-backward compatible fix will be provided in the next major release.

Fixes #3281
  • Loading branch information
Benoit Charbonnier committed Jul 17, 2019
1 parent dd86266 commit ffbb4fd
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 11 deletions.
22 changes: 15 additions & 7 deletions src/modal/modal-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {ContentRef} from '../util/popup';
import {ScrollBar} from '../util/scrollbar';
import {isDefined, isString} from '../util/util';
import {NgbModalBackdrop} from './modal-backdrop';
import {NgbModalOptions} from './modal-config';
import {NgbActiveModal, NgbModalRef} from './modal-ref';
import {NgbModalWindow} from './modal-window';

Expand Down Expand Up @@ -61,7 +62,8 @@ export class NgbModalStack {
}

const activeModal = new NgbActiveModal();
const contentRef = this._getContentRef(moduleCFR, options.injector || contentInjector, content, activeModal);
const contentRef =
this._getContentRef(moduleCFR, options.injector || contentInjector, content, activeModal, options);

let backdropCmptRef: ComponentRef<NgbModalBackdrop> =
options.backdrop !== false ? this._attachBackdrop(moduleCFR, containerEl) : null;
Expand Down Expand Up @@ -124,16 +126,16 @@ export class NgbModalStack {
}

private _getContentRef(
moduleCFR: ComponentFactoryResolver, contentInjector: Injector, content: any,
activeModal: NgbActiveModal): ContentRef {
moduleCFR: ComponentFactoryResolver, contentInjector: Injector, content: any, activeModal: NgbActiveModal,
options: NgbModalOptions): ContentRef {
if (!content) {
return new ContentRef([]);
} else if (content instanceof TemplateRef) {
return this._createFromTemplateRef(content, activeModal);
} else if (isString(content)) {
return this._createFromString(content);
} else {
return this._createFromComponent(moduleCFR, contentInjector, content, activeModal);
return this._createFromComponent(moduleCFR, contentInjector, content, activeModal, options);
}
}

Expand All @@ -154,14 +156,20 @@ export class NgbModalStack {
}

private _createFromComponent(
moduleCFR: ComponentFactoryResolver, contentInjector: Injector, content: any,
context: NgbActiveModal): ContentRef {
moduleCFR: ComponentFactoryResolver, contentInjector: Injector, content: any, context: NgbActiveModal,
options: NgbModalOptions): ContentRef {
const contentCmptFactory = moduleCFR.resolveComponentFactory(content);
const modalContentInjector =
Injector.create({providers: [{provide: NgbActiveModal, useValue: context}], parent: contentInjector});
const componentRef = contentCmptFactory.create(modalContentInjector);
const componentNativeEl = componentRef.location.nativeElement;
if (options.scrollable) {
(componentNativeEl as HTMLElement).classList.add('component-host-scrollable');
}
this._applicationRef.attachView(componentRef.hostView);
return new ContentRef([[componentRef.location.nativeElement]], componentRef.hostView, componentRef);
// FIXME: we should here get rid of the component nativeElement
// and use `[Array.from(componentNativeEl.childNodes)]` instead and remove the above CSS class.
return new ContentRef([[componentNativeEl]], componentRef.hostView, componentRef);
}

private _setAriaHidden(element: Element) {
Expand Down
7 changes: 5 additions & 2 deletions src/modal/modal-window.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
Input,
OnDestroy,
OnInit,
Output
Output,
ViewEncapsulation,
} from '@angular/core';

import {getFocusableBoundaryElements} from '../util/focus-trap';
Expand All @@ -30,7 +31,9 @@ import {ModalDismissReasons} from './modal-dismiss-reasons';
(scrollable ? ' modal-dialog-scrollable' : '')" role="document">
<div class="modal-content"><ng-content></ng-content></div>
</div>
`
`,
encapsulation: ViewEncapsulation.None,
styleUrls: ['./modal.scss']
})
export class NgbModalWindow implements OnInit,
AfterViewInit, OnDestroy {
Expand Down
7 changes: 7 additions & 0 deletions src/modal/modal.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
ngb-modal-window {
.component-host-scrollable {
display: flex;
flex-direction: column;
overflow: hidden;
}
}
12 changes: 10 additions & 2 deletions src/modal/modal.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,9 @@ import {
ViewChild
} from '@angular/core';
import {async, ComponentFixture, TestBed} from '@angular/core/testing';

import {NgbModalConfig} from './modal-config';
import {NgbActiveModal, NgbModal, NgbModalModule, NgbModalRef} from './modal.module';


const NOOP = () => {};

@Injectable()
Expand Down Expand Up @@ -815,6 +813,16 @@ describe('ngb-modal', () => {
fixture.detectChanges();
expect(fixture.nativeElement).not.toHaveModal();
});

it('should add specific styling to content component host', () => {
const modalInstance = fixture.componentInstance.openCmpt(DestroyableCmpt, {scrollable: true});
fixture.detectChanges();
expect(document.querySelector('destroyable-cmpt')).toHaveCssClass('component-host-scrollable');

modalInstance.close();
fixture.detectChanges();
expect(fixture.nativeElement).not.toHaveModal();
});
});

describe('accessibility', () => {
Expand Down

0 comments on commit ffbb4fd

Please sign in to comment.