-
Notifications
You must be signed in to change notification settings - Fork 38
Complaints #47
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Complaints #47
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@preetamozarde3 I haven't been through everything, but have highlighted a few issues here. You might want to use an IDE like VS Code that has linting built in to fix lint issues.
"@angular-devkit/build-angular": "^0.13.0", | ||
"@angular/cli": "^7.0.2", | ||
"@angular/compiler-cli": "^7.0.0", | ||
"@angular/language-service": "^7.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't change anything in package.json
and package-lock.json
import { MatButtonModule } from '@angular/material/button'; | ||
import { MatDialogModule } from '@angular/material'; | ||
import { DetailedComplaintComponent } from './venter/detailed-complaint/detailed-complaint.component'; | ||
import { MatDividerModule } from '@angular/material/divider'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these imports don't belong here. Most of them are already imported in material-angular.module.ts
. Any extra @angular/material
module imports should be placed there.
@@ -100,6 +119,21 @@ import { CardComponent } from './card/card.component'; | |||
HttpClientModule, | |||
FormsModule, | |||
BrowserAnimationsModule, | |||
AgmCoreModule.forRoot({ | |||
apiKey: 'AIzaSyCKNBwrs1UdT2s1jwqOypSzas9Z4s6h4B0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Never commit API keys
- I thought we were not going to use google maps here?
@@ -121,6 +155,10 @@ import { CardComponent } from './card/card.component'; | |||
{ path: 'settings', component: SettingsComponent, data: { state: 'base' } }, | |||
{ path: 'about', component: AboutComponent, data: { state: 'overlay' } }, | |||
|
|||
{ path: 'venter/complaints-home', component: ComplaintsHomeComponent, data: { state: 'base' } }, | |||
{ path: 'venter/file-complaint', component: FileComplaintComponent, data: { state: 'base' } }, | |||
{ path: 'venter/detailed-complaint/:id', component: DetailedComplaintComponent, data: { state: 'base' } }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Place these between login
and feedback
with a whitespace above and below
|
||
GetComplaint(uuid: string): Observable<IComplaint> { | ||
return this.FireGET<IComplaint>(API.Complaint, {uuid: uuid}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really necessary? I can hardly imaging this function being called in more than two places.
<br> | ||
|
||
<a button class="upVote-button" (click)="upVoteComplaint(detailedComplaint.upvoted)"> | ||
UPVOTE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this to a mat button
import { API } from '../../../api'; | ||
import * as moment from 'moment'; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra whitespace
this.statusColor = 'red'; | ||
} else if (this.detailedComplaint.status === 'Resolved') { | ||
this.statusColor = 'green'; | ||
} else if (this.detailedComplaint.status === 'In progress') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above; you could refactor this to a different file altogether
if (this.complaintId) { | ||
this.dataService.FireGET<IComplaint>(API.Complaint, { complaintId: this.complaintId }).subscribe(result => { | ||
this.detailedComplaint = result; | ||
console.log(this.detailedComplaint); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No logging
</g> | ||
<g> | ||
</g> | ||
<g> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove these empty tags
The following changes have been made in this branch: