Handle async option changes on select #91
Conversation
src/app/select/select.directive.ts
Outdated
| Observable.fromEvent($(this.selectElement), 'DOMSubtreeModified') | ||
| .debounceTime(100) | ||
| .subscribe(() => { | ||
| if (this.optionLength !== this.optionElementsLength()) { |
There was a problem hiding this comment.
You could add the returned value by this.optionElementsLength() to variable.
There was a problem hiding this comment.
this.optionElementsLength() has been removed.
| nativeElement = fixture.nativeElement; | ||
| fixture.detectChanges(); | ||
|
|
||
| const optionLength = () => fixture.nativeElement.querySelectorAll('option').length; |
There was a problem hiding this comment.
Could it be this way ?
const optionLength = fixture.nativeElement.querySelectorAll('option').length;
There was a problem hiding this comment.
We can't, because we need to execute the thunk function to get the actual value.
src/app/select/select.directive.ts
Outdated
| this.optionLength = this.optionElementsLength(); | ||
| } | ||
|
|
||
| Observable.fromEvent($(this.selectElement), 'DOMSubtreeModified') |
There was a problem hiding this comment.
this.selectElement is already a JQuery object.
There was a problem hiding this comment.
Sadly, it doesn't work without a JQuery selector around.
Invalid event target.
Did I ever say that I like jQuery? 💥
charpeni
force-pushed
the
async-select-options
branch
from
ed64712
to
5d1a91c
Compare
|
I've pushed a new commit that changed how change event was handled. Now, select keeps |
src/app/select/select.directive.ts
Outdated
| Observable.fromEvent($(this.selectElement), 'DOMSubtreeModified') | ||
| .debounceTime(100) | ||
| .filter((event: JQueryEventObject) => { | ||
| const currentOptions: any = Array.from(event.currentTarget.children); |
There was a problem hiding this comment.
You can remove cast to any by replacing this line with const currentOptions = event.currentTarget.children; and use Array.from(currentOptions).some(...) at Line 232
src/app/select/select.directive.ts
Outdated
| if (prevOptions.length !== currentOptions.length) { | ||
| return true; | ||
| } else { | ||
| return currentOptions.some((option: Element, index) => |
There was a problem hiding this comment.
Type Element become implicit when removing cast to any on currentOptions
|
|
||
| fixture.detectChanges(); | ||
|
|
||
| fixture.whenStable().then(() => { |
There was a problem hiding this comment.
Does not change anything in this case because selectAsyncOptions is not an observable but I was told it will be the case after updating to Angular 4 so no need to change it
src/app/select/select.directive.ts
Outdated
| @@ -96,6 +103,9 @@ export class MzSelectDirective extends HandlePropChanges implements AfterViewIni | |||
| .map((element: HTMLOptionElement) => element.value); | |||
| // setTimeout is needed to this fix to work | |||
| setTimeout(() => this.selectElement.val(selectedOptions)); | |||
|
|
|||
| // prevent close on first multi select change | |||
| this.lastOptions = this.selectElement[0].children; | |||
| this.initOnChange(); | ||
| this.listenOptionChanges(); |
Fixes #88.
There's an issue with select options when they're async, the native select was updated but the changes was not reflected to the select materialized.
This PR partially fix this issue by subscribing to the
DOMSubtreeModifiedevent on select element and count options to notice any changes, if the options count doesn't match, it trigger amaterial_select, then changes are reflected to the select materialized.While this solution works for a majority of use cases without adding any properties to the select, it doesn't cover when options are changed without modifying the length of the options array (ex: trigger translation). These use cases should be handle manually.Edited : Fix now cover every use cases by keeping the last options and compare on every DOM changes.
Also, we can't do a real test case with
Observable.delaybecause of an issue in Zone.js, it should be fixed by the latest release of zone.js. I'll verify that after we've updated our dependencies.