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.

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

Jump to bottom

fix(connected-position-strategy): position change event not emitting for fallback positions #5978

Merged
merged 1 commit into from
Jul 27, 2017
Merged

fix(connected-position-strategy): position change event not emitting for fallback positions #5978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 8 additions & 13 deletions src/lib/autocomplete/autocomplete-trigger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import {Subscription} from 'rxjs/Subscription';
import {merge} from 'rxjs/observable/merge';
import {fromEvent} from 'rxjs/observable/fromEvent';
import {of as observableOf} from 'rxjs/observable/of';
import {RxChain, switchMap, first, filter} from '../core/rxjs/index';
import {RxChain, switchMap, first, filter, map} from '../core/rxjs/index';

/**
* The following style constants are necessary to save here in order
Expand Down Expand Up @@ -380,12 +380,17 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
* stream every time the option list changes.
*/
private _subscribeToClosingActions(): Subscription {
const firstStable = first.call(this._zone.onStable);
const optionChanges = map.call(this.autocomplete.options.changes, () =>
this._positionStrategy.recalculateLastPosition());

// When the zone is stable initially, and when the option list changes...
return RxChain.from(merge(first.call(this._zone.onStable), this.autocomplete.options.changes))
return RxChain.from(merge(firstStable, optionChanges))
// create a new stream of panelClosingActions, replacing any previous streams
// that were created, and flatten it so our stream only emits closing events...
.call(switchMap, () => {
this._resetPanel();
this._resetActiveItem();
this.autocomplete._setVisibility();
return this.panelClosingActions;
})
// when the first closing event occurs...
Expand Down Expand Up @@ -482,14 +487,4 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
this.autocomplete._keyManager.setActiveItem(-1);
}

/**
* Resets the active item and re-calculates alignment of the panel in case its size
* has changed due to fewer or greater number of options.
*/
private _resetPanel() {
this._resetActiveItem();
this._positionStrategy.recalculateLastPosition();
this.autocomplete._setVisibility();
}

}
89 changes: 50 additions & 39 deletions src/lib/core/overlay/position/connected-position-strategy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {OverlayPositionBuilder} from './overlay-position-builder';
import {ConnectedOverlayPositionChange} from './connected-position';
import {Scrollable} from '../scroll/scrollable';
import {Subscription} from 'rxjs/Subscription';
import Spy = jasmine.Spy;
import {ScrollDispatchModule} from '../scroll/index';


Expand Down Expand Up @@ -343,53 +342,65 @@ describe('ConnectedPositionStrategy', () => {
expect(latestCall.args[0] instanceof ConnectedOverlayPositionChange)
.toBe(true, `Expected strategy to emit an instance of ConnectedOverlayPositionChange.`);

it('should pick the fallback position that shows the largest area of the element', () => {
positionBuilder = new OverlayPositionBuilder(viewportRuler);
// If the strategy is re-applied and the initial position would now fit,
// the position change event should be emitted again.
originElement.style.top = '200px';
originElement.style.left = '200px';
strategy.apply(overlayElement);

originElement.style.top = '200px';
originElement.style.right = '25px';
originRect = originElement.getBoundingClientRect();
expect(positionChangeHandler).toHaveBeenCalledTimes(2);

strategy = positionBuilder.connectedTo(
fakeElementRef,
{originX: 'end', originY: 'center'},
{overlayX: 'start', overlayY: 'center'})
.withFallbackPosition(
{originX: 'end', originY: 'top'},
{overlayX: 'start', overlayY: 'bottom'})
.withFallbackPosition(
{originX: 'end', originY: 'top'},
{overlayX: 'end', overlayY: 'top'});
subscription.unsubscribe();
});

strategy.apply(overlayElement);
it('should emit the onPositionChange event even if none of the positions fit', () => {
positionBuilder = new OverlayPositionBuilder(viewportRuler);
originElement.style.bottom = '25px';
originElement.style.right = '25px';

let overlayRect = overlayElement.getBoundingClientRect();
strategy = positionBuilder.connectedTo(
fakeElementRef,
{originX: 'end', originY: 'bottom'},
{overlayX: 'start', overlayY: 'top'})
.withFallbackPosition(
{originX: 'start', originY: 'bottom'},
{overlayX: 'end', overlayY: 'top'});

expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.top));
expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.left));
});
const positionChangeHandler = jasmine.createSpy('positionChangeHandler');
const subscription = strategy.onPositionChange.subscribe(positionChangeHandler);

it('should position a panel properly when rtl', () => {
// must make the overlay longer than the origin to properly test attachment
overlayElement.style.width = `500px`;
originRect = originElement.getBoundingClientRect();
strategy = positionBuilder.connectedTo(
fakeElementRef,
{originX: 'start', originY: 'bottom'},
{overlayX: 'start', overlayY: 'top'})
.withDirection('rtl');
originElement.style.top = '0';
originElement.style.left = '0';
strategy.apply(overlayElement);

// If the strategy is re-applied and the initial position would now fit,
// the position change event should be emitted again.
strategy.apply(overlayElement);
expect(positionChangeHandler).toHaveBeenCalledTimes(2);
expect(positionChangeHandler).toHaveBeenCalled();

subscription.unsubscribe();
});
subscription.unsubscribe();
});

it('should pick the fallback position that shows the largest area of the element', () => {
positionBuilder = new OverlayPositionBuilder(viewportRuler);

originElement.style.top = '200px';
originElement.style.right = '25px';
originRect = originElement.getBoundingClientRect();

strategy = positionBuilder.connectedTo(
fakeElementRef,
{originX: 'end', originY: 'center'},
{overlayX: 'start', overlayY: 'center'})
.withFallbackPosition(
{originX: 'end', originY: 'top'},
{overlayX: 'start', overlayY: 'bottom'})
.withFallbackPosition(
{originX: 'end', originY: 'top'},
{overlayX: 'end', overlayY: 'top'});

strategy.apply(overlayElement);

let overlayRect = overlayElement.getBoundingClientRect();

expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.top));
expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.left));
});

/**
* Run all tests for connecting the overlay to the origin such that first preferred
Expand Down Expand Up @@ -485,7 +496,7 @@ describe('ConnectedPositionStrategy', () => {
let strategy: ConnectedPositionStrategy;

let scrollable: HTMLDivElement;
let positionChangeHandler: Spy;
let positionChangeHandler: jasmine.Spy;
let onPositionChangeSubscription: Subscription;
let positionChange: ConnectedOverlayPositionChange;

Expand Down
10 changes: 5 additions & 5 deletions src/lib/core/overlay/position/connected-position-strategy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,6 @@ export class ConnectedPositionStrategy implements PositionStrategy {
// Save the last connected position in case the position needs to be re-calculated.
this._lastConnectedPosition = pos;

// Notify that the position has been changed along with its change properties.
const scrollableViewProperties = this.getScrollableViewProperties(element);
const positionChange = new ConnectedOverlayPositionChange(pos, scrollableViewProperties);
this._onPositionChange.next(positionChange);

return;
} else if (!fallbackPoint || fallbackPoint.visibleArea < overlayPoint.visibleArea) {
fallbackPoint = overlayPoint;
Expand Down Expand Up @@ -395,6 +390,11 @@ export class ConnectedPositionStrategy implements PositionStrategy {

element.style[verticalStyleProperty] = `${y}px`;
element.style[horizontalStyleProperty] = `${x}px`;

// Notify that the position has been changed along with its change properties.
const scrollableViewProperties = this.getScrollableViewProperties(element);
const positionChange = new ConnectedOverlayPositionChange(pos, scrollableViewProperties);
this._onPositionChange.next(positionChange);
}

/** Returns the bounding positions of the provided element with respect to the viewport. */
Expand Down