-
Notifications
You must be signed in to change notification settings - Fork 2
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
Catalog Update without having to hit bottom of page #2076
Conversation
…ment in CourseProductDetailEnroll_test.js to facilitate splitting tests per render method
…over to a separate block which calls deeprender only. Have to put changes back in to fix teh generated course data and should be set.
…ior, shallowly, ignoring any calls. we otherwise test that these components are working with redux (which is what makes those calls that were previously asserted by sinon. Now I'm taking those as fact since we want to test the behavior of this component with the information provided, this is a shallow render, ignoring those calls (and un-complicating the test)
This reverts commit fd4163f.
# Conflicts: # frontend/public/src/components/CourseProductDetailEnroll.js # frontend/public/src/components/CourseProductDetailEnroll_test.js # frontend/public/src/components/NotificationContainer_test.js # frontend/public/src/containers/UpsellCardApp.js
… loading the second set again
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.
Few minor comments and failing js tests.
{ threshold: 1.0 } | ||
) | ||
this.io.observe(this.container.current) | ||
console.log("gets 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.
let's remove this line
if (!coursesIsLoading) { | ||
if (this.props.courses.length < this.props.coursesCount) { | ||
const response = await getNextCoursePage(this.state.courseQueryPage + 1) | ||
console.log(response) |
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.
and this one
@@ -464,6 +409,10 @@ export class CatalogPage extends React.Component<Props> { | |||
) | |||
} | |||
|
|||
async loadMorePrograms() { |
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.
is this needed?
Closing as this was fixed in #2130 |
What are the relevant tickets?
fixes https://github.com/mitodl/hq/issues/3141
fixes https://github.com/mitodl/hq/issues/2992
fixes https://github.com/mitodl/hq/issues/2910
fixes https://github.com/mitodl/hq/issues/2909
Description (What does it do?)
Rather than relying on the page scroll to load more, there is a chain of requests to fill out the data for the page until it is full, regardless of page movement, once the initial load is complete.
How can this be tested?
Set up data with more than 12 courses and/or programs (ideally both). You will want a few departments loaded as well. Load the catalog and click to filter the list. You should get all cards loading in.