feat: links to cross-listings in modal (#1699)

coursetable · May 14, 2024 · 9c07500 · 9c07500
1 parent 8913fbb
commit 9c07500
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 48 deletions.
diff --git a/frontend/src/components/CourseModal/CourseModal.module.css b/frontend/src/components/CourseModal/CourseModal.module.css
@@ -16,6 +16,12 @@
   margin: 0 0.5rem 0 0;
 }
 
+.crossListingLink,
+.crossListingLink:hover {
+  color: inherit;
+  text-decoration: underline;
+}
+
 .cancelledText {
   color: rgb(255 98 98);
 }

diff --git a/frontend/src/components/CourseModal/CourseModal.tsx b/frontend/src/components/CourseModal/CourseModal.tsx
@@ -1,4 +1,4 @@
-import { useEffect, useState } from 'react';
+import React, { useEffect, useState } from 'react';
 import { Link, useSearchParams } from 'react-router-dom';
 import clsx from 'clsx';
 import { Modal, DropdownButton, Dropdown } from 'react-bootstrap';
@@ -14,7 +14,7 @@ import type { Listings } from '../../generated/graphql-types';
 import type { Season, Crn } from '../../queries/graphql-types';
 import { extraInfo } from '../../utilities/constants';
 import { toSeasonString, truncatedText } from '../../utilities/course';
-import { suspended, useCourseModalLink } from '../../utilities/display';
+import { suspended, createCourseModalLink } from '../../utilities/display';
 import SkillBadge from '../SkillBadge';
 import { TextComponent } from '../Typography';
 import WorksheetToggleButton from '../Worksheet/WorksheetToggleButton';
@@ -164,6 +164,8 @@ function CourseModal() {
   const [view, setView] = useState<'overview' | 'evals'>('overview');
   // Stack for listings that the user has viewed
   const [history, setHistory] = useState<CourseModalHeaderData[]>([]);
+  // This will update when history updates
+  const listing = history[history.length - 1];
   useEffect(() => {
     if (history.length !== 0) return;
     const courseModal = searchParams.get('course-modal');
@@ -175,8 +177,10 @@ function CourseModal() {
       setHistory([listingFromQuery]);
     });
   }, [history.length, searchParams, requestSeasons, courses]);
-  const listing = history[history.length - 1];
-  const backTarget = useCourseModalLink(history[history.length - 2]);
+  const backTarget = createCourseModalLink(
+    history[history.length - 2],
+    searchParams,
+  );
 
   if (!listing) return null;
   const title = `${listing.course_code} ${listing.section.padStart(2, '0')} ${listing.course.title} | CourseTable`;
@@ -241,9 +245,33 @@ function CourseModal() {
               <div className={styles.badges}>
                 <p className={styles.courseCodes}>
                   <TextComponent type="tertiary">
-                    {listing.course.listings
-                      .map((l) => l.course_code)
-                      .join(' • ')}
+                    {listing.course.listings.map((l, i) => (
+                      <React.Fragment key={l.crn}>
+                        {i > 0 && ' • '}
+                        {l.crn === listing.crn ? (
+                          l.course_code
+                        ) : (
+                          <Link
+                            className={styles.crossListingLink}
+                            to={createCourseModalLink(
+                              { crn: l.crn, season_code: listing.season_code },
+                              searchParams,
+                            )}
+                            // We replace instead of pushing to history. I don't
+                            // think navigating between cross-listings should be
+                            // treated as an actual navigation
+                            onClick={() => {
+                              setHistory([
+                                ...history.slice(0, -1),
+                                { ...listing, ...l },
+                              ]);
+                            }}
+                          >
+                            {l.course_code}
+                          </Link>
+                        )}
+                      </React.Fragment>
+                    ))}
                   </TextComponent>
                 </p>
                 {[...listing.course.skills, ...listing.course.areas].map(
@@ -278,7 +306,7 @@ function CourseModal() {
         </Modal.Header>
         {view === 'overview' ? (
           <CourseModalOverview
-            gotoCourse={(l) => {
+            onNavigation={(l) => {
               user.hasEvals ? setView('evals') : setView('overview');
               if (
                 l.crn === listing.crn &&

diff --git a/frontend/src/components/CourseModal/CourseModalOverview.tsx b/frontend/src/components/CourseModal/CourseModalOverview.tsx
@@ -10,10 +10,10 @@ import Spinner from '../Spinner';
 import './react-multi-toggle-override.css';
 
 function CourseModalOverview({
-  gotoCourse,
+  onNavigation,
   header,
 }: {
-  readonly gotoCourse: (x: CourseModalHeaderData) => void;
+  readonly onNavigation: (x: CourseModalHeaderData) => void;
   readonly header: CourseModalHeaderData;
 }) {
   const { user } = useUser();
@@ -46,7 +46,7 @@ function CourseModalOverview({
           <OverviewInfo data={data} />
         </Col>
         <Col md={5} className="px-0 my-0">
-          <OverviewRatings gotoCourse={gotoCourse} data={data} />
+          <OverviewRatings onNavigation={onNavigation} data={data} />
         </Col>
       </Row>
     </Modal.Body>

diff --git a/frontend/src/components/CourseModal/OverviewRatings.tsx b/frontend/src/components/CourseModal/OverviewRatings.tsx
@@ -1,5 +1,5 @@
 import React, { useMemo, useState } from 'react';
-import { Link } from 'react-router-dom';
+import { Link, useSearchParams } from 'react-router-dom';
 import clsx from 'clsx';
 import {
   Row,
@@ -22,7 +22,7 @@ import type {
 import { generateRandomColor } from '../../utilities/common';
 import { ratingColormap, workloadColormap } from '../../utilities/constants';
 import { toSeasonString, isDiscussionSection } from '../../utilities/course';
-import { useCourseModalLink } from '../../utilities/display';
+import { createCourseModalLink } from '../../utilities/display';
 import { RatingBubble } from '../Typography';
 
 import styles from './OverviewRatings.module.css';
@@ -111,25 +111,20 @@ function CourseLink({
   listing,
   course,
   filter,
-  gotoCourse,
+  onNavigation,
 }: {
   readonly listing: SameCourseOrProfOfferingsQuery['self'][0];
   readonly course: RelatedCourseInfoFragment;
   readonly filter: Filter;
-  readonly gotoCourse: (x: CourseModalHeaderData) => void;
+  readonly onNavigation: (x: CourseModalHeaderData) => void;
 }) {
-  const linkTargets = useCourseModalLink(
-    course.listings.map((l) => ({
-      season_code: course.season_code,
-      crn: l.crn,
-    })),
-  );
+  const [searchParams] = useSearchParams();
   // Note, we purposefully use the listing data fetched from GraphQL instead
   // of the static seasons data. This means on navigation we don't have to
   // possibly fetch a new season and cause a loading screen.
   // We have to "massage" this data to fit the flat shape like the one
   // sent by the api. This will be changed.
-  const targetCourses = course.listings.map((l) => ({
+  const targetListings = course.listings.map((l) => ({
     ...l,
     season_code: course.season_code,
     section: course.section,
@@ -143,15 +138,15 @@ function CourseLink({
         : course.course_professors.length === 0
           ? 'TBA'
           : `${course.course_professors[0]!.professor.name}${course.course_professors.length > 1 ? ` +${course.course_professors.length - 1}` : ''}`;
-  if (linkTargets.length === 1) {
+  if (targetListings.length === 1) {
     return (
       <Col
         as={Link}
         xs={5}
         className={clsx(styles.ratingBubble, 'p-0 me-3 text-center')}
-        to={linkTargets[0]!}
+        to={createCourseModalLink(targetListings[0], searchParams)}
         onClick={() => {
-          gotoCourse(targetCourses[0]!);
+          onNavigation(targetListings[0]!);
         }}
       >
         <strong>{toSeasonString(course.season_code)}</strong>
@@ -168,13 +163,13 @@ function CourseLink({
         <Popover id="cross-listing-popover" {...props}>
           <Popover.Body>
             This class has multiple cross-listings:
-            {course.listings.map((l, i) => (
+            {targetListings.map((l, i) => (
               <Link
                 key={i}
                 className="d-block"
-                to={linkTargets[i]!}
+                to={createCourseModalLink(l, searchParams)}
                 onClick={() => {
-                  gotoCourse(targetCourses[i]!);
+                  onNavigation(l);
                 }}
               >
                 {l.course_code === listing.course_code ? (
@@ -192,7 +187,6 @@ function CourseLink({
         as={Button}
         xs={5}
         className={clsx(styles.ratingBubble, 'p-0 me-3 text-center')}
-        to={linkTargets[0]!}
       >
         <strong>{toSeasonString(course.season_code)}</strong>
         <span className={clsx(styles.details, 'mx-auto')}>{extraText}</span>
@@ -232,10 +226,10 @@ function haveSameProfessors(
 }
 
 function OverviewRatings({
-  gotoCourse,
+  onNavigation,
   data,
 }: {
-  readonly gotoCourse: (x: CourseModalHeaderData) => void;
+  readonly onNavigation: (x: CourseModalHeaderData) => void;
   readonly data: SameCourseOrProfOfferingsQuery;
 }) {
   const { user } = useUser();
@@ -309,7 +303,7 @@ function OverviewRatings({
                 listing={listing}
                 course={course}
                 filter={filter}
-                gotoCourse={gotoCourse}
+                onNavigation={onNavigation}
               />
               <RatingNumbers course={course} hasEvals={user.hasEvals} />
             </Row>

diff --git a/frontend/src/utilities/display.tsx b/frontend/src/utilities/display.tsx
@@ -120,32 +120,21 @@ export const scrollToTop: MouseEventHandler = (event) => {
   if (!newPage) window.scrollTo({ top: 0, left: 0 });
 };
 
-function createCourseModalLink(
-  listing: Pick<Listings, 'season_code' | 'crn'>,
+// Please use this instead of creating a new search param. This will preserve
+// existing params.
+export function createCourseModalLink(
+  listing: Pick<Listings, 'season_code' | 'crn'> | undefined,
   searchParams: URLSearchParams,
 ) {
   const newSearch = new URLSearchParams(searchParams);
+  if (!listing) return `?${searchParams.toString()}`;
   newSearch.set('course-modal', `${listing.season_code}-${listing.crn}`);
   return `?${newSearch.toString()}`;
 }
 
-// Please use this instead of creating a new search param. This will preserve
-// existing params.
 export function useCourseModalLink(
   listing: Pick<Listings, 'season_code' | 'crn'> | undefined,
-): string;
-export function useCourseModalLink(
-  listing: Pick<Listings, 'season_code' | 'crn'>[],
-): string[];
-export function useCourseModalLink(
-  listing:
-    | Pick<Listings, 'season_code' | 'crn'>
-    | Pick<Listings, 'season_code' | 'crn'>[]
-    | undefined,
 ) {
   const [searchParams] = useSearchParams();
-  if (!listing) return `?${searchParams.toString()}`;
-  if (Array.isArray(listing))
-    return listing.map((l) => createCourseModalLink(l, searchParams));
   return createCourseModalLink(listing, searchParams);
 }