Skip to content

Conversation

scastillo-jp
Copy link
Collaborator

No description provided.

Comment on lines +1 to +11
<!-- -<div class="d-flex">
- <div
- class="month w-100 text-center align-self-center d-flex flex-column"
- *ngFor="let month of months; let i = index"
- (click)="getMonth(i)"
- [ngClass]="{ active: activeMonth === i }"
- >
- <div class="p-2">{{ month }}</div>
- </div>
-</div>
-->
Copy link
Contributor

Choose a reason for hiding this comment

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

should not you remove this dead code?

}

onChange(monthIndex: number, year: number) {
// tslint:disable-next-line:no-unused-expression
Copy link
Contributor

Choose a reason for hiding this comment

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

no reason to have this line here.

this.changeDate.emit({monthIndex, year});
}

// tslint:disable-next-line:no-unused-expression
Copy link
Contributor

Choose a reason for hiding this comment

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

no reason to have this line here.

Comment on lines +72 to +77
for (let i = 5; i > 0; i--) {
this.years.push(this.model.selectedYearMoment.year() - i);
}
for (let i = 0; i <= 6; i++) {
this.years.push(this.model.selectedYearMoment.year() + i);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What are you trying to do here? is this better?

    const currentYear = this.model.selectedYearMoment.year();
    for (let i = currentYear -5; i < currentYear+5; i++) {
      if( i !== currentYear){ this.years.push(i); }
    }

selectYear(year: number) {
this.isShowYears = false;
this.model.selectedYearMoment = moment().year(year);
this.model.updateYearText(); // in set get aendern
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean?

// in set get aendern

@@ -36,7 +40,7 @@ export class TimeEntriesComponent implements OnInit, OnDestroy {
}

ngOnInit(): void {
this.store.dispatch(new entryActions.LoadEntries(new Date().getMonth() + 1));
// this.store.dispatch(new entryActions.LoadEntries(new Date().getMonth() + 1));
Copy link
Contributor

Choose a reason for hiding this comment

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

can we remove this commented line?

Comment on lines +174 to +176
// getMonth(month: number) {
// this.store.dispatch(new entryActions.LoadEntries(month));
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

can we remove this commented code?


renderYears() {
this.years = [];
for (let i = 5; i > 0; i--) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What are these magic numbers doing here?

Comment on lines +1 to +11
<!-- -<div class="d-flex">
- <div
- class="month w-100 text-center align-self-center d-flex flex-column"
- *ngFor="let month of months; let i = index"
- (click)="getMonth(i)"
- [ngClass]="{ active: activeMonth === i }"
- >
- <div class="p-2">{{ month }}</div>
- </div>
-</div>
-->
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, let's try to avoid pushing commented code. Unless this would be needed somewhere else.

Comment on lines +173 to +176

// getMonth(month: number) {
// this.store.dispatch(new entryActions.LoadEntries(month));
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

The same comment as before. No commented code pushed, please.

@@ -1,23 +1,133 @@
import { Component, OnInit, Output, EventEmitter } from '@angular/core';
import { Component, OnInit, Output, Input, EventEmitter } from '@angular/core';
import * as moment from 'moment';
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to import everything from moment? Or just some utilities.

I'm suggesting this because in front end we need to be careful of what we are importing. Sometimes it impacts to the performance.

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.

4 participants