Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Complaints suggestions #49

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

preetamozarde3
Copy link

The following changes have been made in this branch:

  • A complaint/suggestions module has been added to the app.
  • A complaint/suggestions tab has been added in the navigation bar, on the clicking of which the user is taken to the complaints home page.
  • On this page, the user can:
    • View all the complaints posted in the app.
    • Up vote a complaint if they are in support of it.
    • Subscribe to a particular complaint.
    • Navigate to the complaint details page to view the details of a particular complaint by clicking on the complaint card.
    • Navigate to the file complaints page to post a new complaint by clicking on the 'Vent your issues' button at the top of the page.
  • The file complaints page allows the user to file a new complaint. They can add the following information along with the description to give a more complete account of their issue:
    • Images, suggestions, location details and tags
  • The complaint details page has the following features:
    • The users can view the description, suggestions and location details added by the creator.
    • The users can view the images and tags (if any) associated with the complaint.
    • The users can comment, up vote or subscribe to the complaint if the wish.
  • All the components along with the associated HTML and CSS files for the aforementioned three pages have been added to a new folder named venter.
  • Apart from these additions, additions have been made to the nav menu, data service, interfaces, API and assets.

@pulsejet
Copy link
Collaborator

pulsejet commented Feb 8, 2019

@preetamozarde3 check my review on #47; I had left 40 odd comments on it that'll need to be fixed.

Copy link
Collaborator

@pulsejet pulsejet left a comment

Choose a reason for hiding this comment

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

I haven't tested this yet; but I'll try to do that in the next couple of days. In the meanwhile, other than the mostly minor issues I've pointed out below, you might want to make sure the design is responsive. Particularly, it should work on the following screen sizes (these are the sizes and browsers the rest of the app is being tested on):

  1. 4.7 inch phone (iPhone)
  2. 5.5+ inch phones (general android phones)
  3. 10 inch tablet (iPad, say)
  4. 14+ inch laptops

and must have browser compatibility for

  1. Firefox (Desktop)
  2. Chrome (Desktop)
  3. Edge (Desktop)
  4. Chrome (Android)
  5. Safari (iOS)
  6. IE 11 (best effort)

In general, you shouldn't have issues with JS compatibilty unless you're using some funky ES7 which you might need to polyfill, but CSS behavior can differ seriously between desktop browsers (which are somewhat consistent), Chrome for Android and iOS Safari.

Also, you could take cues from the rest of the app if you're unsure how this is supposed to look in responsive mode. In general, the screen is divided into three parts for desktop and tablet, with the navmenu at the left, a list or editing area at center and detailed information to the right. There are reusable components to make such a layout. More specifically, check the feed, calendar, add-event and update-body components' layouts.

@@ -100,6 +112,7 @@ import { CardComponent } from './card/card.component';
HttpClientModule,
FormsModule,
BrowserAnimationsModule,
MyMaterialClass,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is already imported on line 157

@@ -120,7 +133,6 @@ import { CardComponent } from './card/card.component';
{ path: 'quick-links', component: QuickLinksComponent, data: { state: 'base' } },
{ path: 'settings', component: SettingsComponent, data: { state: 'base' } },
{ path: 'about', component: AboutComponent, data: { state: 'overlay' } },

Copy link
Collaborator

Choose a reason for hiding this comment

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

This whitespace was intentional

<g>
<path d="M0,48v416h512V48H0z M480,432H32V80h448V432z M352,160c0,26.51,21.49,48,48,48s48-21.49,48-48s-21.49-48-48-48
S352,133.49,352,160z M448,400H64l96-256l128,160l64-48L448,400z"/>
</g>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use add_image_placeholder.svg (which already exists in assets) instead of adding a new file

],
providers: [
DataService,
{ provide: RouteReuseStrategy, useClass: CustomReuseStrategy },
LoginActivate,
VenterDataService,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this just below DataService


export interface IPostComment {
text: string;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need separate interfaces for posting. Reuse IComplaintComment.

subscribeToComplaint(complaint_subscribed: number, complaintId: string) {
this.dataService.FireGET<IComplaint>(API.SubscribeToComplaint,
{ complaintId: complaintId, action: complaint_subscribed === 0 ? 1 : 0 }).subscribe(result => {
console.log(result);
Copy link
Collaborator

Choose a reason for hiding this comment

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

No logging to console please.

} else if (complaint.status.toLowerCase() === 'resolved') {
complaint.status_color = 'green';
} else if (complaint.status.toLowerCase() === 'in progress') {
complaint.status_color = 'yellow';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use switch...case here

if (this.complaint.description === '') {
this.venterDataService.getSnackbar('Please enter a description before submitting the complaint!', null);
} else {
this.dataService.FirePOST<IComplaint>(API.Complaints, this.complaint).subscribe(() => {});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Indent

this.editedComment = {} as IPostComment;
/* Get profile if the user is logged in */
if (this.dataService.isLoggedIn()) {
this.getUser();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is still being treated as sync.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just looked through this carefully; if I'm right that userid isn't being used anywhere besides checking if the comment was made by the user, you could let this be the way it is and explicitly initialize userid to null to be sure

})
export class ComplaintsHomeComponent implements OnInit {

public complaints = {} as IComplaint[];
Copy link
Collaborator

Choose a reason for hiding this comment

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

You're basically casting a JSON object to an array here, which works cause the way JS is, but this is terribly confusing. In general, it's better to initialize everything as null (i.e. no initialization, just specify the type) and then put *ngIfs to check if data has been loaded async.

Copy link
Author

Choose a reason for hiding this comment

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

I tried that but I keep getting a TypeError (Cannot find property '...' of undefined)

Copy link
Collaborator

Choose a reason for hiding this comment

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

That means you're trying to access some member for the variable before it is initialized (probably a misplaced ngIf). Before accessing anything in the variable, you need to check if it isn't null, easiest way to do which is add a *ngIf="complaints" to the container in which it is being accessed.

mat-icon-button>
<mat-icon>delete</mat-icon>
</button>
<button *ngIf="complaintComment.commented_by.id==userid"
Copy link
Collaborator

Choose a reason for hiding this comment

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

=== not ==


public complaints = {} as IComplaint[];
public myComplaints = {} as IComplaint[];
public userid: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where is userid being used in this component?

Copy link
Collaborator

pulsejet commented May 1, 2020

Codacy Here is an overview of what got changed by this pull request:

Issues
======
- Added 140
           

See the complete overview on Codacy


.complaint-tag {
padding: 1% 0;
background-color: yellowgreen
Copy link
Collaborator

Choose a reason for hiding this comment

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

}

.complaint-upvote-active {
color: #536dfe;
Copy link
Collaborator

Choose a reason for hiding this comment

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

color: white;
padding: 0 2.5%;
font-weight: bold;
text-transform: uppercase;
Copy link
Collaborator

Choose a reason for hiding this comment

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


.display-image {
max-height: 400px;
max-width: 400px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

id: string;
time: string;
text: string;
commented_by: IUserProfile;
Copy link
Collaborator

Choose a reason for hiding this comment

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


.footer {
position: relative;
text-align: center;
Copy link
Collaborator

Choose a reason for hiding this comment

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

}

.complaint-location-details {
padding-top: 2%;
Copy link
Collaborator

Choose a reason for hiding this comment

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


.complaint-button {
color: black;
background-color: yellow;
Copy link
Collaborator

Choose a reason for hiding this comment

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

font-size: 18px;
border-radius: 0 3px 3px 0;
user-select: none;
background-color: rgba(0,0,0,0.8);
Copy link
Collaborator

Choose a reason for hiding this comment

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


.toolbar {
padding: 150px 0;
background-color: #536dfe;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@@ -162,6 +162,52 @@ export interface IUserTagCategory {
tags: IUserTag[];
}

export interface IComplaintTag {
id: string;
tag_uri: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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


.active,
.dot:hover {
background-color: #717171;
Copy link
Collaborator

Choose a reason for hiding this comment

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

/* The dots/bullets/indicators */

.dot {
cursor: pointer;
Copy link
Collaborator

Choose a reason for hiding this comment

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


.dot {
cursor: pointer;
height: 15px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

color: black;
background-color: yellow;
padding: 10px 50px;
font-weight: bold;
Copy link
Collaborator

Choose a reason for hiding this comment

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

}

.no-complaints-image {
width: 50%;
Copy link
Collaborator

Choose a reason for hiding this comment

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

* {box-sizing: border-box}
body {margin:0}
.mySlides {display: none;}
img {vertical-align: middle; height: 20%;}
Copy link
Collaborator

Choose a reason for hiding this comment

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

* {box-sizing: border-box}
body {margin:0}
.mySlides {display: none;}
img {vertical-align: middle; height: 20%;}
Copy link
Collaborator

Choose a reason for hiding this comment

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

.status {
float: right;
background-color: red;
border-width: medium;
Copy link
Collaborator

Choose a reason for hiding this comment

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


export interface IComplaint {
id: string;
created_by: IUserProfile;
Copy link
Collaborator

Choose a reason for hiding this comment

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

UserTagsReach: 'api/user-tags/reach'
UserTagsReach: 'api/user-tags/reach',

Complaints: 'api/venter/complaints',
Copy link
Collaborator

Choose a reason for hiding this comment

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

margin: 0 2px;
background-color: #bbb;
border-radius: 50%;
display: inline-block;
Copy link
Collaborator

Choose a reason for hiding this comment

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

users_up_voted: IUserProfile[];
images: string[];
comments: IComplaintComment[];
vote_count: number;
Copy link
Collaborator

Choose a reason for hiding this comment

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


* {box-sizing: border-box}
body {margin:0}
.mySlides {display: none; padding-right: 5%;}
Copy link
Collaborator

Choose a reason for hiding this comment

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

report_date: string;
reported_date: string;
status: string;
status_color: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

2 participants