Skip to content

Development #1

Merged
merged 11 commits into from May 1, 2019
Merged

Development #1

merged 11 commits into from May 1, 2019

Conversation

ema14006
Copy link
Contributor

Added hover effects, header adjustments depending if you are logged in, and changed graph button on statistics page

Copy link
Contributor

@eal13009 eal13009 left a comment

Choose a reason for hiding this comment

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

Most of it looks good, would like to either clarify or change the isLoggedIn function

@@ -23,6 +23,17 @@ export class HeaderComponent implements OnInit {
resetGenAndVisualize() {
this.resetAndRedirect('/visualize-all');
}
isLoggedIn()
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the logic behind this function? A user being logged in shouldn't be dictated by the router url, as anyone can have a link to a URL and suddenly be shown icons as if they were logged in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified isLoggedIn() to check for auth_token availability instead. Let me know if that is a good solution or not

@ema14006
Copy link
Contributor Author

Found logic error with redirect, don't merge please

@eal13009 eal13009 merged commit 06ac7bc into production May 1, 2019
Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants