-
Notifications
You must be signed in to change notification settings - Fork 56
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
Fixes #66 added user/progress mock endpoint #129
Fixes #66 added user/progress mock endpoint #129
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.
Thanks for cleaning up the diff and formatting. There's just a few small improvements and I think it's ready to merge!
$('.container').css('height', $(window).height()); | ||
$(window).on('resize', function () { | ||
$('.container').css('height', $(window).height()); |
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.
These lines were deleted in a merge, but git must not have detected it, so please remove from this PR.
if (!response.ok) { | ||
const message = `An error has occured: ${response.status}`; | ||
console.error(message) | ||
return 1 |
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.
Not sure if you missed the other PR comment but the return 1
won't work because the next step is expecting a userProgress
object
Hi @ayushjain94 did you have a chance to look at the last review for this? |
Closed due to inactivity. |
Creating a new pull request as I made a force push in previous PR and it got closed. I have incorporated all the changes suggested in the reviews.
closes #66