-
Notifications
You must be signed in to change notification settings - Fork 781
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
Highlight tab of side-bar when moving to challenge components #3531
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.
@gautamjajoo can you please fix the error that I am getting which compiling this branch.The issue is here and here Please see the screenshot attached below for the ref of the error-
Also, the highlight feature is working for except for one case i.e when you are directed to all challenges
page after logging in. Please see the recording below for ref -
Screen.Recording.2021-07-14.at.7.50.49.PM.mov
@@ -88,7 +99,7 @@ export class HeaderStaticComponent implements OnInit, OnDestroy { | |||
private apiService: ApiService, | |||
@Inject(DOCUMENT) private document: Document | |||
) { | |||
this.authState = authService.authState; |
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.
[Question] Why are we removing this line?
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.
This was removed by mistake. Sorry!
} | ||
} | ||
}); | ||
this.isChallengeComponent = isNaN(this.router.url.split('/')[length]); |
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.
Try isNaN(parseInt(this.router.url.split('/')[length]));
in order to get rid of the compilation error
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.
Yes, actually I didn't notice that error.
@Kajol-Kumari Done the changes. Also, the highlight feature is working fine for the above case as well. I accidently removed the |
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.
LGTM 👍
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.
LGTM
@Ram81 @Kajol-Kumari
tab-highlight.mp4