Skip to content

Conversation

@pavelsm-ctco
Copy link

No description provided.

loadChildren: () =>
import('./posts/posts.routes').then((mod) => mod.routes),
providers: [
PostsService,
Copy link

@up-ctc up-ctc May 29, 2025

Choose a reason for hiding this comment

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

Services that have @Injectable({ providedIn: 'root'}) should be not provided separately. Providing in root means service Available on whole app level and is alive while app is alive. This will also guarantee that data is persisted when you are switching between routes.

In current approach there are actually 2 PostsService. One on App level and then this route component has its own.

Copy link
Author

Choose a reason for hiding this comment

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

thx. removed.

<mat-label>Message</mat-label>
<textarea matInput formControlName="message" rows="3">
</textarea>
@if (message.hasError("minlength") || message.hasError("maxlength") || message.hasError("required")) {
Copy link

Choose a reason for hiding this comment

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

No need to change it now, just suggestion for future ->
It's always better to extract more complex logic to separate component function because it is easier to unit test it (rather than test this behavior in component), but it is also correct. Just hint from my work experience. :)

}

ngOnInit(): void {
this.doSubmitSubscription = this.doSubmit$.subscribe(this.onSubmit.bind(this));
Copy link

Choose a reason for hiding this comment

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

Just FYI another good solution how to take care of unsubscribes (its useful if you have several subscriptions).

`private subscriptions= new Subscription();

this.subscriptions.add (
this.doSubmit$.subscribe(...
);

ngOnDestroy() {
this.subscriptions.unsubscribe()}

Then you will be able to handle several subscriptions at once.

Copy link
Author

Choose a reason for hiding this comment

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

thx, i've changed to the suggested approach. In our project i have not see this approach. It is a lot more similar .NET winforms events.


@Component({
selector: 'app-posts-list',
imports: [NgIf, MatCardModule, RouterLink, MatButtonModule],
Copy link

Choose a reason for hiding this comment

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

If you use new @if / @for syntax instead of old approach directives then no need to import ngIf or Common module:
https://angular.dev/essentials/templates#control-flow-with-if-and-for


private breakpointObserver = inject(BreakpointObserver);

isHandset$: Observable<boolean> = this.breakpointObserver.observe(Breakpoints.Handset)
Copy link

Choose a reason for hiding this comment

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

is it used somewhere?

Copy link
Author

Choose a reason for hiding this comment

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

removed

) {}

ngOnInit(): void {
if (this.el) {
Copy link

Choose a reason for hiding this comment

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

this.el should always be truthy.

Copy link
Author

Choose a reason for hiding this comment

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

simplified IF statement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants