-
-
Notifications
You must be signed in to change notification settings - Fork 866
Fix #4478 : Added lazy loading to the All Challenges page #4484
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
base: master
Are you sure you want to change the base?
Fix #4478 : Added lazy loading to the All Challenges page #4484
Conversation
@PrasadCodesML hey! I was working on the issue please make sure you communicate with others too next time. |
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.
Hey @PrasadCodesML , Thanks for the PR. Can you please remove the formatting changes to the lines that are not required?
$timeout(vm.checkVisibility, 500); | ||
}); | ||
} | ||
})(); |
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 add a new line here.
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 i will modify those changes
@@ -1,13 +1,27 @@ | |||
<section class="ev-sm-container ev-view challenge-container"> | |||
<!-- ongoing challenges --> | |||
<div class="challenge-page-title" id = "ongoing-challenges"><strong class="text-med-black fs-18">Ongoing Challenges</strong></div> | |||
<div class="challenge-page-title" id="ongoing-challenges"> |
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.
What are we changing here?
<div ng-if="challengeList.noneCurrentChallenge">None</div> | ||
<div class="row"> | ||
<div class="col s12 m3" ng-repeat="challenge in challengeList.currentList"><a class="ev-card-hover" ui-sref="web.challenge-main.challenge-page({challengeId:challenge.id})"> | ||
<div class="col s12 m3" ng-repeat="challenge in challengeList.currentList"> |
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.
What are we changing here?
}); | ||
}); | ||
}); |
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.
Add a new line here.
Hey @PrasadCodesML , Can you please fix the travis build? |
Yes working on it. |
…rasadCodesML/EvalAI into Fix/Add_lazy_loading_on_challenge_page
…rasadCodesML/EvalAI into Fix/Add_lazy_loading_on_challenge_page
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4484 +/- ##
==========================================
- Coverage 72.93% 67.98% -4.95%
==========================================
Files 83 20 -63
Lines 5368 3692 -1676
==========================================
- Hits 3915 2510 -1405
+ Misses 1453 1182 -271
... and 64 files with indirect coverage changes
... and 64 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Hey, @RishabhJain2018 the Travis build is fixed |
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.
A couple of questions:
- Why are we using pagination?
- There are some changes like adding the loader, modifying css, etc. Why are those needed?
- Can we use any library to implement lazy-loading?
- Can you populate the database with a couple of more challenges, and then show the network tab from the Google Developer console? I am not sure if the challenges are loaded according to the viewport.
Why are we using pagination? Why are loaders and CSS modifications needed? Can we use lazy-loading libraries? Can you populate the database with a couple of more challenges, and then show the network tab from the Google Developer console? I am not sure if the challenges are loaded according to the viewport. |
This PR fixes the all challenges page by loading content using lazy loading method as mentioned in the issue #4478
Untitled.video.-.Made.with.Clipchamp.2.mp4
Changes After Adding Lazy Loading are as follows
1. frontend/src/views/web/challenge-list.html
Scroll Event Listener for Lazy Loading
2. frontend/src/js/controllers/challengeListCtrl.js
New
imageLoaded
Method AddedLazy Loading Optimization
Pagination Logic Modified
loadMore()
function.3. frontend\tests\controllers-test\challengeListCtrl.test.js
Updated the tests folder for proper build changed
The above code is doing visbility based loading that is why there are some factor to consider initially all the ongoing, upcoming and past challenges div's are visible on the screen which will make a api call and fetch the challenge data but since initially all the 3 are visible it will load the fastest to fetch api first, even though i have included a function to
loadSequentially()
but the api calling speed of past and upcoming functions are fast as compared to the ongoing function so sometimes it might happen some of the past challenges are being loaded.However, this can be fixed by increasing the visibility time in
checkVisibility()
but if there are no ongoing challenges it will take more time to load the upcoming and past challenges.One of the solution to this problem can be introducing 3 tabs on the All challenges page to top right by clicking on that particular time(ongoing, upcoming or past) we can load challenges accordingly.
Let me know if any changes are need, or if you prefer to solve the problem in a different way...