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

Course details #41

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/components/listItems/assignmentListItem.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
@import 'variables';

.name {
color: $text-color;
padding-top: 1rem;
margin: 0;
}

.dueDate {
color: $text-color
}
23 changes: 23 additions & 0 deletions src/components/listItems/assignmentListItem.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import React from 'react'
import { Assignment } from 'devu-shared-modules'

import styles from './assignmentListItem.scss'
import { prettyPrintDate } from 'utils/date.utils'

import ListItemWrapper from 'components/shared/layouts/listItemWrapper'

type Props = {
assignment: Assignment,
courseId: string
}

const AssignmentListItem = ({ assignment, courseId }: Props) => (
<ListItemWrapper
to={`/courses/${courseId}/assignments/${assignment.id}`}
tag={`${courseId}-${assignment.id}`}>
<h3 className={styles.name}>{assignment.name}</h3>
Copy link
Member

Choose a reason for hiding this comment

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

Under normal circumstances I don't think you'd want to use a header in a list item, even if the sizing is correct. It make screen readers go wonky when you're misusing "headers"

<p className={styles.dueDate}>Due At: {prettyPrintDate(assignment.dueDate)}</p>
Copy link
Member

Choose a reason for hiding this comment

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

Might we want to include an assignments description here? Maybe make it ellipse out after 1 line? At that point it might not even be useful

Maybe 3 or so lines?

</ListItemWrapper>
)

export default AssignmentListItem
55 changes: 55 additions & 0 deletions src/components/pages/courseDetailPage.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
@import 'variables';

.subtitle {
display: flex;
align-items: baseline;
margin-bottom: 1rem;

h3 {
margin: 0 1rem 0 0;
}
}

.buttons {
margin-bottom: 1rem;

& .button:not(:last-child) {
margin-right: 1rem;
}
}

.assignmentsHeader {
display: flex;
justify-content:space-between;

& .dropdowns {
display: flex;

& .dropdown {
width: 300px;
}

& .dropdown:not(:last-child) {
margin-right: 1rem;
}
}
}

@media (max-width: 850px) {
.assignmentsHeader {
display: block;

& .dropdowns {
margin-bottom: 1rem;
}
}
}

@media (max-width: 600px) {
.subtitle {
display: block;
h3 {
margin-bottom: .5rem;
}
}
}
143 changes: 141 additions & 2 deletions src/components/pages/courseDetailPage.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,146 @@
import React from 'react'
import React, { useEffect, useState } from 'react'
import { Assignment, Course, UserCourse } from 'devu-shared-modules'

import PageWrapper from 'components/shared/layouts/pageWrapper'
import LoadingOverlay from 'components/shared/loaders/loadingOverlay'
import Dropdown, { Option } from 'components/shared/inputs/dropdown'
import Button from 'components/shared/inputs/button'
import AssignmentListItem from 'components/listItems/assignmentListItem'

const CourseDetailPage = ({}) => <PageWrapper>Course Detail</PageWrapper>
import ErrorPage from './errorPage'
import styles from './courseDetailPage.scss'

import RequestService from 'services/request.service'
import LocalStorageService from 'services/localStorage.service'
import { Link, useParams } from 'react-router-dom'
import { prettyPrintDate } from 'utils/date.utils'
import { useAppSelector } from 'redux/hooks'

const ORDER_BY_STORAGE_KEY = 'course_assignments_order_by'
const GROUP_BY_STORAGE_KEY = 'course_assignments_group_by'

type OrderBy = 'activeDueDate' | 'dueDate' | 'active' | 'assignmentName'
type GroupBy = 'id' | 'category' | 'name' | 'dueDate' | 'active'

const orderByOptions: Option<OrderBy>[] = [
{ label: 'Default', value: 'activeDueDate' },
{ label: 'Due Date', value: 'dueDate' },
{ label: 'Active', value: 'active' },
{ label: 'Name', value: 'assignmentName' },
]

const groupByOptions: Option<GroupBy>[] = [
{ label: 'Defualt', value: 'id' },
{ label: 'Category', value: 'category' },
{ label: 'Name', value: 'name' },
{ label: 'Due Date', value: 'dueDate' },
{ label: 'Active', value: 'active' }
]

type Params = {
courseId: string
}

const CourseDetailPage = () => {
const defaultOrderBy = LocalStorageService.get<OrderBy>(ORDER_BY_STORAGE_KEY) || 'activeDueDate'
const defaultGroupBy = LocalStorageService.get<GroupBy>(GROUP_BY_STORAGE_KEY) || 'id'

const [loading, setLoading] = useState(true);
const [course, setCourse] = useState({} as Course);
const [userCourse, setUserCourse] = useState<UserCourse>({} as UserCourse);
const [assignments, setAssignments] = useState(new Array<Assignment>());
const [error, setError] = useState(null);
const [orderBy, setOrderBy] = useState<OrderBy>(defaultOrderBy);
const [groupBy, setGroupBy] = useState<GroupBy>(defaultGroupBy);

const { courseId } = useParams() as Params;

useEffect(() => {
fetchCourse(orderBy, groupBy);
}, []);

const userId = useAppSelector((store) => store.user.id);

const fetchCourse = async (orderBy: OrderBy, groupBy: GroupBy) => {
try {
const pageCourse = await RequestService.get(`/api/courses/${courseId}`);
setCourse(pageCourse);
setAssignments(await RequestService.get(`/api/assignments?orderBy=${orderBy}?groupBy=${groupBy}`));

// Find the users relationship to the course by calling all user-course objects, and seeing if any have the user/course id.
const userCourses = await RequestService.get<UserCourse[]>(`/api/user-courses`);
const uCourse = userCourses.filter(u => u.courseId === parseInt(courseId) && u.userId === userId).shift()
Copy link
Member

Choose a reason for hiding this comment

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

Firstly, you don't actually need the

u.userId === userId 

part since this endpoint only returns the current user's user-courses.

Secondly, I think this would be a good opportunity to add a query parameter to just look up the one we want rather than grabbing the whole list and shrunking it down

/api/user-courses?courseId=123

if (uCourse) { setUserCourse(uCourse); }
}
catch(error) {
setError(error)
} finally {
setLoading(false);
}
}

const handleFilterChange = (updatedOrderBy: OrderBy) => {
setOrderBy(updatedOrderBy)
fetchCourse(updatedOrderBy, groupBy)

LocalStorageService.set(ORDER_BY_STORAGE_KEY, updatedOrderBy)
}

const handleGroupByChange = (updatedGroupBy: GroupBy) => {
setGroupBy(updatedGroupBy)
fetchCourse(orderBy, updatedGroupBy)

LocalStorageService.set(GROUP_BY_STORAGE_KEY, updatedGroupBy)
}

if (loading) return <LoadingOverlay delay={250} />
if (error) return <ErrorPage error={error} />

const defaultOrderByOption = orderByOptions.find((o) => o.value === orderBy)
const defaultGroupByOption = groupByOptions.find((o) => o.value === groupBy)

return (
<PageWrapper>
<header>
<h1>{course.name}</h1>
<div className={styles.subtitle}>
<h3>{course.number} | {course.semester} ({prettyPrintDate(course.startDate)}-{prettyPrintDate(course.endDate)})</h3>
Copy link
Member

Choose a reason for hiding this comment

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

This is a proper use of a header.

And with this you might be able to see why having a bunch of these would make using a screen reader a bit harder to use

{userCourse.level === "instructor" &&
<Link to={`${courseId}/update`}><Button>Update Course</Button></Link>
}
</div>
</header>
{userCourse &&
<div className={styles.buttons}>
<Link className={styles.button} to={`${courseId}/assignments`}><Button>Gradebook</Button></Link>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not entirely sure this is the correct link, but I was having issues finding a grade book page

<Link className={styles.button} to={`${courseId}/roster`}><Button>Roster</Button></Link>
</div>
}
<div>
<div className={styles.assignmentsHeader}>
<h2>Assignments</h2>
<div className={styles.dropdowns}>
<Dropdown
label='Group By'
className={styles.dropdown}
options={groupByOptions}
onChange={handleGroupByChange}
defaultOption={defaultGroupByOption}
/>
<Dropdown
label='Order By'
className={styles.dropdown}
options={orderByOptions}
onChange={handleFilterChange}
defaultOption={defaultOrderByOption}
/>
</div>
</div>
{assignments.map((assignment) => (
<AssignmentListItem key={assignment.id} assignment={assignment} courseId={course.id?.toString() || ''} />
))}
</div>
</PageWrapper>
)
}
export default CourseDetailPage
6 changes: 1 addition & 5 deletions src/components/pages/userCoursesListPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,7 @@ const UserCoursesListPage = () => {
</div>
</div>
{userCourses.map((userCourse) => (
<UserCourseListItem
key={userCourse.courseId}
userCourse={userCourse}
course={courses[userCourse.courseId || '']}
/>
<UserCourseListItem key={userCourse.courseId} userCourse={userCourse} course={courses[userCourse.courseId || '']} />
))}
</PageWrapper>
)
Expand Down