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.

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

fix(option): emit selection change when selection is changed #1334

Merged
merged 14 commits into from
Apr 8, 2019
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
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
23 changes: 15 additions & 8 deletions src/framework/theme/components/select/option.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
OnDestroy,
Output,
} from '@angular/core';
import { Observable, Subject } from 'rxjs';
import { convertToBoolProperty } from '../helpers';
import { NbSelectComponent } from './select.component';

Expand Down Expand Up @@ -52,10 +53,18 @@ export class NbOptionComponent<T> implements OnDestroy {
}

/**
* Fires value on click.
* Fires value when option selection change.
* */
@Output() selectionChange: EventEmitter<NbOptionComponent<T>> = new EventEmitter();

/**
* Fires when option clicked
*/
private click$: Subject<NbOptionComponent<T>> = new Subject<NbOptionComponent<T>>();
get click(): Observable<NbOptionComponent<T>> {
return this.click$.asObservable();
}

selected: boolean = false;
disabled: boolean = false;
private alive: boolean = true;
Expand Down Expand Up @@ -84,19 +93,17 @@ export class NbOptionComponent<T> implements OnDestroy {
return this.parent.multiple;
}

@HostBinding('class.selected')
get selectedClass(): boolean {
@HostBinding('class.selected') get selectedClass(): boolean {
yggg marked this conversation as resolved.
Show resolved Hide resolved
return this.selected;
}

@HostBinding('class.disabled')
get disabledClass(): boolean {
@HostBinding('class.disabled') get disabledClass(): boolean {
return this.disabled;
}

@HostListener('click')
onClick() {
this.selectionChange.emit(this);
this.click$.next(this);
}

select() {
Expand All @@ -115,10 +122,10 @@ export class NbOptionComponent<T> implements OnDestroy {
* Also Angular can call writeValue on destroyed view (select implements ControlValueAccessor).
* angular/angular#27803
* */
if (this.alive) {
if (this.alive && this.selected !== isSelected) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

since instance variable is called selected why don't we call local variable same selected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in 0a89ed2

this.selected = isSelected;
this.selectionChange.emit(this);
this.cd.markForCheck();
}
}
}

54 changes: 29 additions & 25 deletions src/framework/theme/components/select/select.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
} from '@angular/core';
import { ControlValueAccessor, NG_VALUE_ACCESSOR } from '@angular/forms';
import { merge, Observable, Subject } from 'rxjs';
import { takeWhile } from 'rxjs/operators';
import { startWith, switchMap, takeWhile } from 'rxjs/operators';

import {
NbAdjustableConnectedPositionStrategy,
Expand Down Expand Up @@ -252,10 +252,14 @@ export class NbSelectComponent<T> implements OnInit, AfterViewInit, AfterContent
*/
overlayPosition: NbPosition = '' as NbPosition;

/**
* Stream of events that will fire when one of the options fire selectionChange event.
* */
/* @deprecated */
protected selectionChange$: Subject<NbOptionComponent<T>> = new Subject();
Copy link
Collaborator

Choose a reason for hiding this comment

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

since this is internal change there is no need to deprecate it, we can just break it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in 5ca3837

/**
* Stream of events that will fire when one of the options is clicked.
* @deprecated
* Use nb-select (selected) binding to track selection change and <nb-option (click)=""> to track option click.
* @breaking-change 4.0.0
**/
readonly selectionChange: Observable<NbOptionComponent<T>> = this.selectionChange$.asObservable();

protected ref: NbOverlayRef;
Expand Down Expand Up @@ -339,7 +343,7 @@ export class NbSelectComponent<T> implements OnInit, AfterViewInit, AfterContent

this.subscribeOnTriggers();
this.subscribeOnPositionChange();
this.subscribeOnSelectionChange();
this.subscribeOnOptionClick();
}

ngOnDestroy() {
Expand Down Expand Up @@ -391,7 +395,7 @@ export class NbSelectComponent<T> implements OnInit, AfterViewInit, AfterContent
/**
* Selects option or clear all selected options if value is null.
* */
protected handleSelect(option: NbOptionComponent<T>) {
protected handleOptionClick(option: NbOptionComponent<T>) {
if (option.value) {
this.selectOption(option);
} else {
Expand Down Expand Up @@ -501,19 +505,24 @@ export class NbSelectComponent<T> implements OnInit, AfterViewInit, AfterContent
});
}

protected subscribeOnSelectionChange() {
this.subscribeOnOptionsSelectionChange();
protected subscribeOnOptionClick() {
/**
* If the user changes provided options list in the runtime we have to handle this
* and resubscribe on options selection changes event.
* Otherwise, the user will not be able to select new options.
* */
this.options.changes
.subscribe(() => this.subscribeOnOptionsSelectionChange());

this.selectionChange
.pipe(takeWhile(() => this.alive))
.subscribe((option: NbOptionComponent<T>) => this.handleSelect(option));
.pipe(
startWith(this.options),
switchMap((options: QueryList<NbOptionComponent<T>>) => {
return merge(...options.map(option => option.click));
}),
takeWhile(() => this.alive),
)
.subscribe((clickedOption: NbOptionComponent<T>) => {
this.handleOptionClick(clickedOption);
this.selectionChange$.next(clickedOption);
});
}

protected getContainer() {
Expand Down Expand Up @@ -546,20 +555,21 @@ export class NbSelectComponent<T> implements OnInit, AfterViewInit, AfterContent
throw new Error('Can\'t assign array if select is not marked as multiple');
}

this.cleanSelection();
const previouslySelectedOptions = this.selectionModel;
this.selectionModel = [];

if (isArray) {
(<T[]> value).forEach((option: T) => this.selectValue(option));
} else {
this.selectValue(<T> value);
}

this.cd.markForCheck();
}
// find options which were selected before and trigger deselect
previouslySelectedOptions
.filter((option: NbOptionComponent<T>) => !this.selectionModel.includes(option))
.forEach((option: NbOptionComponent<T>) => option.deselect());

protected cleanSelection() {
this.selectionModel.forEach((option: NbOptionComponent<T>) => option.deselect());
this.selectionModel = [];
this.cd.markForCheck();
}

/**
Expand Down Expand Up @@ -587,10 +597,4 @@ export class NbSelectComponent<T> implements OnInit, AfterViewInit, AfterContent
protected isClickedWithinComponent($event: Event) {
return this.hostRef.nativeElement === $event.target || this.hostRef.nativeElement.contains($event.target as Node);
}

protected subscribeOnOptionsSelectionChange() {
merge(...this.options.map(it => it.selectionChange))
.pipe(takeWhile(() => this.alive))
.subscribe((change: NbOptionComponent<T>) => this.selectionChange$.next(change));
}
}
Loading