Skip to content

Commit

Permalink
refactor(material/paginator): remove TypeScript mixin usages
Browse files Browse the repository at this point in the history
Replaces the final usage of TypeScript mixins in the paginator.
  • Loading branch information
crisbeto committed Feb 15, 2024
1 parent ba57666 commit e13ece1
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 18 deletions.
29 changes: 13 additions & 16 deletions src/material/paginator/paginator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ import {
booleanAttribute,
numberAttribute,
} from '@angular/core';
import {MatFormField, MatFormFieldAppearance} from '@angular/material/form-field';
import {HasInitialized, MatOption, mixinInitialized, ThemePalette} from '@angular/material/core';
import {MatOption, ThemePalette} from '@angular/material/core';
import {MatSelect} from '@angular/material/select';
import {MatIconButton} from '@angular/material/button';
import {MatTooltip} from '@angular/material/tooltip';
import {Subscription} from 'rxjs';
import {MatFormField, MatFormFieldAppearance} from '@angular/material/form-field';
import {Observable, ReplaySubject, Subscription} from 'rxjs';
import {MatPaginatorIntl} from './paginator-intl';

/** The default page size if there is no page size and there are no provided page size options. */
Expand Down Expand Up @@ -90,10 +90,6 @@ export const MAT_PAGINATOR_DEFAULT_OPTIONS = new InjectionToken<MatPaginatorDefa
'MAT_PAGINATOR_DEFAULT_OPTIONS',
);

// Boilerplate for applying mixins to _MatPaginatorBase.
/** @docs-private */
const _MatPaginatorMixinBase = mixinInitialized(class {});

let nextUniqueId = 0;

/**
Expand All @@ -115,18 +111,16 @@ let nextUniqueId = 0;
standalone: true,
imports: [MatFormField, MatSelect, MatOption, MatIconButton, MatTooltip],
})
export class MatPaginator
extends _MatPaginatorMixinBase
implements OnInit, OnDestroy, HasInitialized
{
export class MatPaginator implements OnInit, OnDestroy {
/** If set, styles the "page size" form field with the designated style. */
_formFieldAppearance?: MatFormFieldAppearance;

/** ID for the DOM node containing the paginator's items per page label. */
readonly _pageSizeLabelId = `mat-paginator-page-size-label-${nextUniqueId++}`;

private _initialized: boolean;
private _intlChanges: Subscription;
private _isInitialized = false;
private _initializedStream = new ReplaySubject<void>(1);

/** Theme color to be used for the underlying form controls. */
@Input() color: ThemePalette;
Expand Down Expand Up @@ -196,12 +190,14 @@ export class MatPaginator
/** Displayed set of page size options. Will be sorted and include current page size. */
_displayedPageSizeOptions: number[];

/** Emits when the paginator is initialized. */
initialized: Observable<void> = this._initializedStream;

constructor(
public _intl: MatPaginatorIntl,
private _changeDetectorRef: ChangeDetectorRef,
@Optional() @Inject(MAT_PAGINATOR_DEFAULT_OPTIONS) defaults?: MatPaginatorDefaultOptions,
) {
super();
this._intlChanges = _intl.changes.subscribe(() => this._changeDetectorRef.markForCheck());

if (defaults) {
Expand All @@ -228,12 +224,13 @@ export class MatPaginator
}

ngOnInit() {
this._initialized = true;
this._isInitialized = true;
this._updateDisplayedPageSizeOptions();
this._markInitialized();
this._initializedStream.next();
}

ngOnDestroy() {
this._initializedStream.complete();
this._intlChanges.unsubscribe();
}

Expand Down Expand Up @@ -337,7 +334,7 @@ export class MatPaginator
* the page size is an option and that the list is sorted.
*/
private _updateDisplayedPageSizeOptions() {
if (!this._initialized) {
if (!this._isInitialized) {
return;
}

Expand Down
5 changes: 3 additions & 2 deletions tools/public_api_guard/material/paginator.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@

import { ChangeDetectorRef } from '@angular/core';
import { EventEmitter } from '@angular/core';
import { HasInitialized } from '@angular/material/core';
import * as i0 from '@angular/core';
import * as i1 from '@angular/material/button';
import * as i2 from '@angular/material/select';
import * as i3 from '@angular/material/tooltip';
import { InjectionToken } from '@angular/core';
import { MatFormFieldAppearance } from '@angular/material/form-field';
import { Observable } from 'rxjs';
import { OnDestroy } from '@angular/core';
import { OnInit } from '@angular/core';
import { Optional } from '@angular/core';
Expand All @@ -33,7 +33,7 @@ export const MAT_PAGINATOR_INTL_PROVIDER: {
export function MAT_PAGINATOR_INTL_PROVIDER_FACTORY(parentIntl: MatPaginatorIntl): MatPaginatorIntl;

// @public
export class MatPaginator extends _MatPaginatorMixinBase implements OnInit, OnDestroy, HasInitialized {
export class MatPaginator implements OnInit, OnDestroy {
constructor(_intl: MatPaginatorIntl, _changeDetectorRef: ChangeDetectorRef, defaults?: MatPaginatorDefaultOptions);
_changePageSize(pageSize: number): void;
color: ThemePalette;
Expand All @@ -45,6 +45,7 @@ export class MatPaginator extends _MatPaginatorMixinBase implements OnInit, OnDe
hasNextPage(): boolean;
hasPreviousPage(): boolean;
hidePageSize: boolean;
initialized: Observable<void>;
// (undocumented)
_intl: MatPaginatorIntl;
lastPage(): void;
Expand Down

3 comments on commit e13ece1

@mi-po
Copy link

@mi-po mi-po commented on e13ece1 Feb 23, 2024

Choose a reason for hiding this comment

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

shoudn't be there
this._initializedStream.complete();
after
this._initializedStream.next();
?

because current behaviour is different as it was before, this is probably breaking change

in mixinInitialized is comment
/** Emits and completes the subscriber stream (should only emit once). */

@crisbeto
Copy link
Member Author

Choose a reason for hiding this comment

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

There's a _initializedStream.complete in ngOnDestroy. Emitting and then completely it immediately seemed a bit weird and none of our internal tests were affected by it so I figured that it's not particularly breaking.

@mi-po
Copy link

@mi-po mi-po commented on e13ece1 Feb 23, 2024

Choose a reason for hiding this comment

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

I'm using something like this
concat(this.paginator.initialized, ......)
when this.paginator.initialized doesn't complete, it stucks, calling complete in ngOnDestroy is too late in this case
but ok, probably I should change my code...

Please sign in to comment.