Skip to content
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

Closed
wants to merge 41 commits into from

Conversation

JenniWhitman
Copy link
Contributor

@JenniWhitman JenniWhitman commented Jan 30, 2024

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.

jenniw and others added 30 commits December 5, 2023 09:29
…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)
JenniWhitman and others added 10 commits January 22, 2024 12:55
# 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
@JenniWhitman JenniWhitman marked this pull request as ready for review February 9, 2024 14:00
@annagav annagav self-requested a review February 9, 2024 14:26
@annagav annagav self-assigned this Feb 9, 2024
Copy link
Contributor

@annagav annagav left a 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")
Copy link
Contributor

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)
Copy link
Contributor

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() {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this needed?

@annagav
Copy link
Contributor

annagav commented Feb 13, 2024

It says the page is unresponsive.
Screenshot 2024-02-13 at 9 02 17 AM

This happened only after I created over 12 items for the page.

@JenniWhitman
Copy link
Contributor Author

Closing as this was fixed in #2130

@JenniWhitman JenniWhitman deleted the jw/catalog-update branch May 14, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants