Skip to content

Commit

Permalink
Fix mobile counts on catalog (#2114)
Browse files Browse the repository at this point in the history 
* Smaller callback change to facilitate things loading quicklier

* fix duplication of last item

* change how we display mobile counts on CatalogPage.js

* split into two functions and modified

* fmt

* test fix
  • Loading branch information
@JenniWhitman
JenniWhitman authored Feb 27, 2024
1 parent 2309e05 commit ab37475
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 41 deletions.
50 changes: 21 additions & 29 deletions frontend/public/src/containers/pages/CatalogPage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,36 +372,28 @@ export class CatalogPage extends React.Component<Props> {
/**
* Returns the number of courseRuns or programs based on the selected catalog tab.
*/
renderNumberOfCatalogItems() {
const { coursesCount, programsCount, departments } = this.props
if (
this.state.tabSelected === PROGRAMS_TAB &&
this.state.selectedDepartment === ALL_DEPARTMENTS
) {
return programsCount
} else if (
this.state.tabSelected === PROGRAMS_TAB &&
this.state.selectedDepartment !== ALL_DEPARTMENTS
) {
return departments.find(
department => department.name === this.state.selectedDepartment
).programs
}
if (
this.state.tabSelected === COURSES_TAB &&
this.state.selectedDepartment === ALL_DEPARTMENTS
) {
renderNumberOfCatalogCourses() {
const { coursesCount, departments } = this.props
if (this.state.selectedDepartment === ALL_DEPARTMENTS) {
return coursesCount
} else if (
this.state.tabSelected === COURSES_TAB &&
this.state.selectedDepartment !== ALL_DEPARTMENTS
) {
} else if (this.state.selectedDepartment !== ALL_DEPARTMENTS) {
return departments.find(
department => department.name === this.state.selectedDepartment
).courses
}
}

renderNumberOfCatalogPrograms() {
const { programsCount, departments } = this.props
if (this.state.selectedDepartment === ALL_DEPARTMENTS) {
return programsCount
} else if (this.state.selectedDepartment !== ALL_DEPARTMENTS) {
return departments.find(
department => department.name === this.state.selectedDepartment
).programs
} else return 0
}

/**
* Renders a single course catalog card.
* @param {CourseDetailWithRuns} course The course instance used to populate the card.
Expand Down Expand Up @@ -594,7 +586,7 @@ export class CatalogPage extends React.Component<Props> {
>
Courses{" "}
<div className="product-number d-inline-block d-sm-none">
({this.state.filteredCourses.length})
({this.renderNumberOfCatalogCourses()})
</div>
</button>
</div>
Expand All @@ -604,9 +596,7 @@ export class CatalogPage extends React.Component<Props> {
? "selected-tab"
: "unselected-tab"
} ${
this.state.filteredPrograms.length
? ""
: "display-none"
this.props.programsCount ? "" : "display-none"
}`}
>
<button
Expand All @@ -616,7 +606,7 @@ export class CatalogPage extends React.Component<Props> {
>
Programs{" "}
<div className="product-number d-inline-block d-sm-none">
({this.state.filteredPrograms.length})
({this.renderNumberOfCatalogPrograms()})
</div>
</button>
</div>
Expand All @@ -639,7 +629,9 @@ export class CatalogPage extends React.Component<Props> {
<h2>
{/* Hidden on small screens. */}
{/* Could add logic to display only "course" if only 1 course is showing. */}
{this.renderNumberOfCatalogItems()}{" "}
{this.state.tabSelected === PROGRAMS_TAB
? this.renderNumberOfCatalogPrograms()
: this.renderNumberOfCatalogCourses()}{" "}
{this.state.tabSelected}
</h2>
</CSSTransition>
Expand Down
25 changes: 13 additions & 12 deletions frontend/public/src/containers/pages/CatalogPage_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ describe("CatalogPage", function() {
expect(JSON.stringify(inner.state().allCoursesRetrieved)).equals(
JSON.stringify(courses)
)
expect(inner.instance().renderNumberOfCatalogItems()).equals(1)
expect(inner.instance().renderNumberOfCatalogCourses()).equals(1)
})

it("updates state from changeSelectedTab when selecting program tab", async () => {
Expand Down Expand Up @@ -247,7 +247,7 @@ describe("CatalogPage", function() {
expect(JSON.stringify(inner.state().allProgramsRetrieved)).equals(
JSON.stringify(programs)
)
expect(inner.instance().renderNumberOfCatalogItems()).equals(5)
expect(inner.instance().renderNumberOfCatalogPrograms()).equals(5)
})

it("renders catalog department filter for courses and programs tabs", async () => {
Expand Down Expand Up @@ -554,14 +554,14 @@ describe("CatalogPage", function() {
JSON.stringify(courses)
)
// Number of catalog items should match the number of visible courses.
expect(inner.instance().renderNumberOfCatalogItems()).equals(3)
expect(inner.instance().renderNumberOfCatalogCourses()).equals(3)

// Select a department to filter by.
inner.instance().changeSelectedDepartment("History", "courses")
// Confirm the state updated to reflect the selected department.
expect(inner.state().selectedDepartment).equals("History")
// Confirm the number of catalog items updated to reflect the items filtered by department.
expect(inner.instance().renderNumberOfCatalogItems()).equals(2)
expect(inner.instance().renderNumberOfCatalogCourses()).equals(2)
// Confirm the courses filtered are those which have a department name matching the selected department.
expect(JSON.stringify(inner.state().filteredCourses)).equals(
JSON.stringify([course2, course3])
Expand Down Expand Up @@ -622,7 +622,7 @@ describe("CatalogPage", function() {
JSON.stringify(courses)
)
// one shows visually, but the total is 2
expect(inner.instance().renderNumberOfCatalogItems()).equals(2)
expect(inner.instance().renderNumberOfCatalogCourses()).equals(2)
expect(inner.state().courseQueryPage).equals(1)

// Mock the second page of course API results.
Expand Down Expand Up @@ -651,7 +651,7 @@ describe("CatalogPage", function() {
expect(JSON.stringify(inner.state().filteredCourses)).equals(
JSON.stringify([displayedCourse, displayedCourse])
)
expect(inner.instance().renderNumberOfCatalogItems()).equals(2)
expect(inner.instance().renderNumberOfCatalogCourses()).equals(2)
})

it("do not load more at the bottom of the courses catalog page if next page is null", async () => {
Expand Down Expand Up @@ -692,7 +692,7 @@ describe("CatalogPage", function() {
expect(JSON.stringify(inner.state().allCoursesRetrieved)).equals(
JSON.stringify(courses)
)
expect(inner.instance().renderNumberOfCatalogItems()).equals(1)
expect(inner.instance().renderNumberOfCatalogCourses()).equals(1)
expect(inner.state().courseQueryPage).equals(1)

// Simulate the user reaching the bottom of the catalog page.
Expand All @@ -707,7 +707,7 @@ describe("CatalogPage", function() {
expect(JSON.stringify(inner.state().filteredCourses)).equals(
JSON.stringify(courses)
)
expect(inner.instance().renderNumberOfCatalogItems()).equals(1)
expect(inner.instance().renderNumberOfCatalogCourses()).equals(1)
})

it("do not load more at the bottom of the courses catalog page if isLoadingMoreItems is true", async () => {
Expand Down Expand Up @@ -745,7 +745,7 @@ describe("CatalogPage", function() {
expect(JSON.stringify(inner.state().filteredCourses)).equals(
JSON.stringify(courses)
)
expect(inner.instance().renderNumberOfCatalogItems()).equals(1)
expect(inner.instance().renderNumberOfCatalogCourses()).equals(1)
expect(inner.state().courseQueryPage).equals(1)

// Simulate the user reaching the bottom of the catalog page.
Expand All @@ -764,7 +764,7 @@ describe("CatalogPage", function() {
expect(JSON.stringify(inner.state().filteredCourses)).equals(
JSON.stringify(courses)
)
expect(inner.instance().renderNumberOfCatalogItems()).equals(1)
expect(inner.instance().renderNumberOfCatalogCourses()).equals(1)
})

it("load more at the bottom of the programs catalog page, all departments filter", async () => {
Expand Down Expand Up @@ -821,7 +821,7 @@ describe("CatalogPage", function() {
JSON.stringify(programs)
)
// While there is only one showing, there are still 2 total. The total should be shown.
expect(inner.instance().renderNumberOfCatalogItems()).equals(2)
expect(inner.instance().renderNumberOfCatalogPrograms()).equals(2)
expect(inner.state().programQueryPage).equals(1)

// Mock the second page of program API results.
Expand Down Expand Up @@ -851,7 +851,8 @@ describe("CatalogPage", function() {
expect(JSON.stringify(inner.state().filteredPrograms)).equals(
JSON.stringify([displayedProgram, displayedProgram])
)

// This should still be 2 because we haven't changed the filter - no matter if one or two have loaded, there are 2
expect(inner.instance().renderNumberOfCatalogItems()).equals(2)
expect(inner.instance().renderNumberOfCatalogPrograms()).equals(2)
})
})

0 comments on commit ab37475

Please sign in to comment.