Angular2: Change detection timing for an auto-scroll directive

Question

I've been working on a simple auto-scroll directive for chat-display:

@Directive({
    selector: "[autoScroll]"
})
export class AutoScroll {
    @Input() inScrollHeight;
    @Input() inClientHeight;

    @HostBinding("scrollTop") outScrollTop;

    ngOnChanges(changes: {[propName: string]: SimpleChange}) {
        if (changes["inScrollHeight"] || changes["inClientHeight"]) {
            this.scroll();
        }
    };

    scroll() {
        this.outScrollTop = this.inScrollHeight - this.inClientHeight;
    };
}

This directive will work when I've set enableProdMode() and when the ChangeDetectionStrategy is set to default, but when in "dev mode" I get an exception. I can set the ChangeDetectionStrategy to onPush, in that case the exception doesn't occur but the scroll will lag behind.

Is there a way to better structure this code so that Dom will be updated then the Scroll function can be called? I've tried setTimeout() but that makes the delay worse, tried using ChangeDetectorRef and subscribing to the observable to trigger markForCheck(). Using ngAfterViewChecked() causes browser crashes.

@Component({
    selector: "chat-display",
    template: `
            <div class="chat-box" #this [inScrollHeight]="this.scrollHeight" [inClientHeight]="this.clientHeight" autoScroll>
                <p *ngFor="#msg of messages | async | messageFilter:username:inSelectedTarget:inTargetFilter:inDirectionFilter" [ngClass]="msg.type">{{msg.message}}</p>
            </div>
       `,
    styles: [`.whisper {
            color: rosybrown;
        }`],
    directives: [NgClass, AutoScroll],
    pipes: [AsyncPipe, MessageFilterPipe],
    changeDetection: ChangeDetectionStrategy.OnPush
})
export class ChatDisplay implements OnInit {

    username: string;
    @Input() inSelectedTarget: string;
    @Input() inTargetFilter: boolean;
    @Input() inDirectionFilter: boolean;

    messages: Observable<ChatType[]>;

    constructor(private socketService_: SocketService, private authService_: AuthService) {
        this.username = this.authService_.username;
    };

    ngOnInit() {
    }

}

This is the the exception that is triggered when in dev mode:

EXCEPTION: Expression 'this.scrollHeight in ChatDisplay@1:40' has changed after it was checked. Previous value: '417'. Current value: '420' in [this.scrollHeight in ChatDisplay@1:40] angular2.dev.js (23083,9)

Carraway

Would be interesting what "an exception" actually is. – Paderewski

It's when change detection is happening twice in dev mode, and expression "this.scrollHeight" is different between checks. Like the observable is pushing messages to the display, and this increases scrollHeight, so it is different between checks, I think that is what is happening. – Carraway

Can you please add the exact error message to your question? – Paderewski

Where do you subscrube to the scroll event? What is calling scroll()? – Paderewski

Added the exception message. The scroll happens when the directive detects any changes in the input properties in the ngOnChanges lifecycle hook. – Carraway

Ah, missed that. Weird. I'd assumed this to work as well. – Paderewski

It does work when "enableProdMode()", exactly as expected. It's just in dev mode that is doesn't. I was thinking this could give issues in other areas if change detection was not being handled properly, i.e. not get the timing of events correct. – Carraway

There should be a way to make it work in devMode as well. Only working in prodMode is smelly. – Paderewski

Answer 1

I found one way to solve this, it involves dividing the chat display into two separate components and use content projection. So there is a flow of changes from parent to child, and not having two functionalities in the same component with one triggering changes in the other. I can used the default changeDetectionStrategy without getting exceptions in dev mode.

@Component({
    selector: "chat-display",
    template: `
    <auto-scroll-display>
        <chat-message *ngFor="#chat of chats | async | messageFilter:username:inSelectedTarget:inTargetFilter:inDirectionFilter" [message]="chat.message" [type]="chat.type"></chat-message>
    </auto-scroll-display>
    `,
    directives: [NgClass, AutoScrollComponent, ChatMessageComponent],
    pipes: [AsyncPipe, MessageFilterPipe]
})
export class ChatDisplay implements OnInit { /* unchanged code */ }

The auto-scroll directive is identical to original post, was trying to figure out if there was a way to combine the directive functionality into the component. It's just acting as a container now.

@Component({
    selector: "auto-scroll-display",
    template: `
    <div #this class="chat-box" [inScrollHeight]="this.scrollHeight" [inClientHeight]="this.clientHeight" autoScroll>
        <ng-content></ng-content>
    </div>
    `,
    directives: [AutoscrollDirective]
})
export class AutoScrollComponent{ }

Here's a github link with working code, link.

Carraway

Answer 2

Is there a way to better structure this code so that DOM will be updated then the Scroll function can be called?

The DOM should be updated before ngAfterViewChecked() is called. See if something like this works:

ngOnChanges(changes: {[propName: string]: SimpleChange}) {
    // detect the change here
    if (changes["inScrollHeight"] || changes["inClientHeight"]) {
        this.scrollAfterDomUpdates = true;
    }
};
ngAfterViewChecked() {
    // but scroll here, after the DOM was updated
    if(this.scrollAfterDomUpdates) {
       this.scrollAfterDomUpdates = false;
       this.scroll();
    }
}

If that doesn't work, try wrapping the call to scroll in a setTimeout:

    if(this.scrollAfterDomUpdates) {
       this.scrollAfterDomUpdates = false;
       this.setTimeout( _ => this.scroll());
    }

Insightful

Hey I tried something like that but it still gives the exceptions about property bindings changing between rounds of change detection. I went and made a simplified project trying to get something like that to work, link. Here is an implementation with content-projection along the lines you are suggesting, link. – Carraway