Skip to content

chore(overlay): add attach method to PositionStrategy interface #6505

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Aug 22, 2017
Merged

chore(overlay): add attach method to PositionStrategy interface #6505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f26ff7d
chore(overlay): add `attach` method to PositionStrategy interface
jelbourn Aug 16, 2017
f8051fb
Use overlay element from attach
jelbourn Aug 17, 2017
fade053
fix some comments
jelbourn Aug 17, 2017
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
6 changes: 5 additions & 1 deletion src/cdk/overlay/overlay-ref.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ export class OverlayRef implements PortalHost {
attach(portal: Portal<any>): any {
let attachResult = this._portalHost.attach(portal);

if (this._state.positionStrategy) {
this._state.positionStrategy.attach(this);
}

// Update the pane element with the given state configuration.
this._updateStackingOrder();
this.updateSize();
Expand Down Expand Up @@ -146,7 +150,7 @@ export class OverlayRef implements PortalHost {
/** Updates the position of the overlay based on the position strategy. */
updatePosition() {
if (this._state.positionStrategy) {
this._state.positionStrategy.apply(this._pane);
this._state.positionStrategy.apply();
}
}

Expand Down
11 changes: 8 additions & 3 deletions src/cdk/overlay/overlay.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,9 +532,14 @@ class OverlayTestModule { }
class OverlayContainerThemingTestModule { }

class FakePositionStrategy implements PositionStrategy {
apply(element: Element): Promise<null> {
element.classList.add('fake-positioned');
return Promise.resolve(null);
element: HTMLElement;

apply(): void {
this.element.classList.add('fake-positioned');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test that this gets executed at the proper time?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't follow what you mean; the existing test for this still applies- is there something that isn't covered?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, didn't know that we had something in place already.

}

attach(overlayRef: OverlayRef) {
this.element = overlayRef.overlayElement;
}

dispose() {}
Expand Down
90 changes: 60 additions & 30 deletions src/cdk/overlay/position/connected-position-strategy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {ConnectedOverlayPositionChange} from './connected-position';
import {Scrollable} from '../scroll/scrollable';
import {Subscription} from 'rxjs/Subscription';
import {ScrollDispatchModule} from '../scroll/index';
import {OverlayRef} from '../overlay-ref';


// Default width and height of the overlay and origin panels throughout these tests.
Expand Down Expand Up @@ -142,7 +143,8 @@ describe('ConnectedPositionStrategy', () => {
{originX: 'start', originY: 'bottom'},
{overlayX: 'start', overlayY: 'top'});

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

let overlayRect = overlayElement.getBoundingClientRect();
expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.bottom));
Expand All @@ -166,7 +168,8 @@ describe('ConnectedPositionStrategy', () => {
{originX: 'end', originY: 'center'},
{overlayX: 'start', overlayY: 'center'});

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

let overlayRect = overlayElement.getBoundingClientRect();
expect(Math.floor(overlayRect.top)).toBe(Math.floor(originCenterY - (OVERLAY_HEIGHT / 2)));
Expand All @@ -188,7 +191,8 @@ describe('ConnectedPositionStrategy', () => {
{originX: 'end', originY: 'top'},
{overlayX: 'end', overlayY: 'bottom'});

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

let overlayRect = overlayElement.getBoundingClientRect();
expect(Math.floor(overlayRect.bottom)).toBe(Math.floor(originRect.top));
Expand All @@ -210,7 +214,8 @@ describe('ConnectedPositionStrategy', () => {
{originX: 'start', originY: 'bottom'},
{overlayX: 'end', overlayY: 'top'});

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

let overlayRect = overlayElement.getBoundingClientRect();

Expand All @@ -234,7 +239,8 @@ describe('ConnectedPositionStrategy', () => {
{overlayX: 'start', overlayY: 'bottom'});

// This should apply the fallback position, as the original position won't fit.
strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

// Now make the overlay small enough to fit in the first preferred position.
overlayElement.style.height = '15px';
Expand All @@ -260,7 +266,8 @@ describe('ConnectedPositionStrategy', () => {
{originX: 'start', originY: 'top'},
{overlayX: 'start', overlayY: 'bottom'});

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();
strategy.recalculateLastPosition();

let overlayRect = overlayElement.getBoundingClientRect();
Expand All @@ -279,7 +286,8 @@ describe('ConnectedPositionStrategy', () => {
{overlayX: 'start', overlayY: 'top'})
.withDirection('rtl');

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

let overlayRect = overlayElement.getBoundingClientRect();
expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.bottom));
Expand All @@ -294,7 +302,8 @@ describe('ConnectedPositionStrategy', () => {
{overlayX: 'start', overlayY: 'top'});

strategy.withOffsetX(10);
strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

let overlayRect = overlayElement.getBoundingClientRect();
expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.top));
Expand All @@ -309,7 +318,8 @@ describe('ConnectedPositionStrategy', () => {
{overlayX: 'start', overlayY: 'top'});

strategy.withOffsetY(50);
strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

let overlayRect = overlayElement.getBoundingClientRect();
expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.top + 50));
Expand All @@ -334,7 +344,8 @@ describe('ConnectedPositionStrategy', () => {
const positionChangeHandler = jasmine.createSpy('positionChangeHandler');
const subscription = strategy.onPositionChange.subscribe(positionChangeHandler);

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

const latestCall = positionChangeHandler.calls.mostRecent();

Expand All @@ -346,7 +357,8 @@ describe('ConnectedPositionStrategy', () => {
// the position change event should be emitted again.
originElement.style.top = '200px';
originElement.style.left = '200px';
strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

expect(positionChangeHandler).toHaveBeenCalledTimes(2);

Expand All @@ -369,7 +381,8 @@ describe('ConnectedPositionStrategy', () => {
const positionChangeHandler = jasmine.createSpy('positionChangeHandler');
const subscription = strategy.onPositionChange.subscribe(positionChangeHandler);

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

expect(positionChangeHandler).toHaveBeenCalled();

Expand All @@ -394,7 +407,8 @@ describe('ConnectedPositionStrategy', () => {
{originX: 'end', originY: 'top'},
{overlayX: 'end', overlayY: 'top'});

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

let overlayRect = overlayElement.getBoundingClientRect();

Expand All @@ -415,7 +429,8 @@ describe('ConnectedPositionStrategy', () => {
{originX: 'start', originY: 'bottom'},
{overlayX: 'start', overlayY: 'top'});

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

let overlayRect = overlayElement.getBoundingClientRect();
expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect!.bottom));
Expand All @@ -428,7 +443,8 @@ describe('ConnectedPositionStrategy', () => {
{originX: 'end', originY: 'center'},
{overlayX: 'start', overlayY: 'center'});

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

let overlayRect = overlayElement.getBoundingClientRect();
expect(Math.floor(overlayRect.top)).toBe(Math.floor(originCenterY! - (OVERLAY_HEIGHT / 2)));
Expand All @@ -441,7 +457,8 @@ describe('ConnectedPositionStrategy', () => {
{originX: 'start', originY: 'bottom'},
{overlayX: 'end', overlayY: 'top'});

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

let overlayRect = overlayElement.getBoundingClientRect();

Expand All @@ -455,7 +472,8 @@ describe('ConnectedPositionStrategy', () => {
{originX: 'end', originY: 'top'},
{overlayX: 'end', overlayY: 'bottom'});

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

let overlayRect = overlayElement.getBoundingClientRect();
expect(Math.round(overlayRect.bottom)).toBe(Math.round(originRect!.top));
Expand All @@ -468,7 +486,8 @@ describe('ConnectedPositionStrategy', () => {
{originX: 'center', originY: 'bottom'},
{overlayX: 'center', overlayY: 'top'});

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

let overlayRect = overlayElement.getBoundingClientRect();
expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect!.bottom));
Expand All @@ -481,7 +500,8 @@ describe('ConnectedPositionStrategy', () => {
{originX: 'center', originY: 'center'},
{overlayX: 'center', overlayY: 'center'});

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();

let overlayRect = overlayElement.getBoundingClientRect();
expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect!.top));
Expand Down Expand Up @@ -526,7 +546,7 @@ describe('ConnectedPositionStrategy', () => {

strategy.withScrollableContainers([
new Scrollable(new FakeElementRef(scrollable), null!, null!, null!)]);

strategy.attach(fakeOverlayRef(overlayElement));
positionChangeHandler = jasmine.createSpy('positionChangeHandler');
onPositionChangeSubscription = strategy.onPositionChange.subscribe(positionChangeHandler);
});
Expand All @@ -538,7 +558,7 @@ describe('ConnectedPositionStrategy', () => {
});

it('should not have origin or overlay clipped or out of view without scroll', () => {
strategy.apply(overlayElement);
strategy.apply();

expect(positionChangeHandler).toHaveBeenCalled();
positionChange = positionChangeHandler.calls.mostRecent().args[0];
Expand All @@ -552,7 +572,7 @@ describe('ConnectedPositionStrategy', () => {

it('should evaluate if origin is clipped if scrolled slightly down', () => {
scrollable.scrollTop = 10; // Clip the origin by 10 pixels
strategy.apply(overlayElement);
strategy.apply();

expect(positionChangeHandler).toHaveBeenCalled();
positionChange = positionChangeHandler.calls.mostRecent().args[0];
Expand All @@ -566,7 +586,7 @@ describe('ConnectedPositionStrategy', () => {

it('should evaluate if origin is out of view and overlay is clipped if scrolled enough', () => {
scrollable.scrollTop = 31; // Origin is 30 pixels, move out of view and clip the overlay 1px
strategy.apply(overlayElement);
strategy.apply();

expect(positionChangeHandler).toHaveBeenCalled();
positionChange = positionChangeHandler.calls.mostRecent().args[0];
Expand All @@ -580,7 +600,7 @@ describe('ConnectedPositionStrategy', () => {

it('should evaluate the overlay and origin are both out of the view', () => {
scrollable.scrollTop = 61; // Scroll by overlay height + origin height + 1px buffer
strategy.apply(overlayElement);
strategy.apply();

expect(positionChangeHandler).toHaveBeenCalled();
positionChange = positionChangeHandler.calls.mostRecent().args[0];
Expand Down Expand Up @@ -626,7 +646,8 @@ describe('ConnectedPositionStrategy', () => {
{originX: 'start', originY: 'top'},
{overlayX: 'start', overlayY: 'top'});

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();
expect(overlayElement.style.left).toBeTruthy();
expect(overlayElement.style.right).toBeFalsy();
});
Expand All @@ -637,7 +658,8 @@ describe('ConnectedPositionStrategy', () => {
{originX: 'end', originY: 'top'},
{overlayX: 'end', overlayY: 'top'});

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();
expect(overlayElement.style.right).toBeTruthy();
expect(overlayElement.style.left).toBeFalsy();
});
Expand All @@ -653,7 +675,8 @@ describe('ConnectedPositionStrategy', () => {
)
.withDirection('rtl');

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();
expect(overlayElement.style.right).toBeTruthy();
expect(overlayElement.style.left).toBeFalsy();
});
Expand All @@ -665,7 +688,8 @@ describe('ConnectedPositionStrategy', () => {
{overlayX: 'end', overlayY: 'top'}
).withDirection('rtl');

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();
expect(overlayElement.style.left).toBeTruthy();
expect(overlayElement.style.right).toBeFalsy();
});
Expand All @@ -679,7 +703,8 @@ describe('ConnectedPositionStrategy', () => {
{overlayX: 'start', overlayY: 'top'}
);

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();
expect(overlayElement.style.top).toBeTruthy();
expect(overlayElement.style.bottom).toBeFalsy();
});
Expand All @@ -691,7 +716,8 @@ describe('ConnectedPositionStrategy', () => {
{overlayX: 'start', overlayY: 'bottom'}
);

strategy.apply(overlayElement);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();
expect(overlayElement.style.bottom).toBeTruthy();
expect(overlayElement.style.top).toBeFalsy();
});
Expand Down Expand Up @@ -741,3 +767,7 @@ function createOverflowContainerElement() {
class FakeElementRef implements ElementRef {
constructor(public nativeElement: HTMLElement) { }
}

function fakeOverlayRef(overlayElement: HTMLElement) {
return {overlayElement} as OverlayRef;
}
Loading