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

feat(components): Create LessonTabs #2394

Merged
38 changes: 38 additions & 0 deletions __tests__/__snapshots__/storyshots.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -11096,6 +11096,44 @@ exports[`Storyshots Components/LessonLayout With Text 1`] = `
</div>
`;

exports[`Storyshots Components/LessonTabs Lesson Tabs 1`] = `
<div
className="navCard__tabsNav"
>
<div
className="navCard__tabsNav__nav"
>
<a
className="navCard__tabsNav__nav__item--inactive nav-pills"
href="/curriculum/js0"
onClick={[Function]}
onMouseEnter={[Function]}
onTouchStart={[Function]}
>
CHALLENGES
</a>
<a
className="navCard__tabsNav__nav__item nav-pills"
href="/exercises/js0"
onClick={[Function]}
onMouseEnter={[Function]}
onTouchStart={[Function]}
>
EXERCISES
</a>
<a
className="navCard__tabsNav__nav__item--inactive nav-pills"
href="/curriculum/js0/mentor"
onClick={[Function]}
onMouseEnter={[Function]}
onTouchStart={[Function]}
>
MENTOR EXERCISES
</a>
</div>
</div>
`;

exports[`Storyshots Components/LessonTitleCard Basic 1`] = `
<div
className="card shadow-sm mt-3 col-12 px-0 pt-3 border-0"
Expand Down
2 changes: 1 addition & 1 deletion __tests__/pages/exercises/[lessonSlug].test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('Exercises page', () => {

screen.getByRole('link', { name: 'CHALLENGES' })
screen.getByRole('link', { name: 'EXERCISES' })
screen.getByRole('link', { name: 'LESSONS' })
screen.getByRole('link', { name: 'LESSON' })
})

test('Renders exercise card with the skip, previous, and exit buttons', async () => {
Expand Down
35 changes: 35 additions & 0 deletions components/LessonTabs/LessonTabs.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import React from 'react'
import { CURRICULUM_PATH } from '../../constants'
import NavCard from '../NavCard'

type Lesson = {
title: string
docUrl?: string | null
slug: string
}

type Props = {
lesson: Lesson
activeTab: 'lesson' | 'challenges' | 'exercises' | 'mentor exercises'
flacial marked this conversation as resolved.
Show resolved Hide resolved
}

const LessonTabs = ({ lesson, activeTab }: Props) => {
const tabs = [
...(lesson.docUrl ? [{ text: 'lesson', url: lesson.docUrl }] : []),
anthonykhoa marked this conversation as resolved.
Show resolved Hide resolved
{ text: 'challenges', url: `${CURRICULUM_PATH}/${lesson.slug}` },
{ text: 'exercises', url: `/exercises/${lesson.slug}` },
Copy link
Member

Choose a reason for hiding this comment

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

Might as well add this path to the constants.

Copy link
Member Author

@flacial flacial Oct 4, 2022

Choose a reason for hiding this comment

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

The exercises shouldn't be under /exercises but something like /curriculum/js0/exercises.

Copy link
Member

@SlyBouhafs SlyBouhafs Oct 5, 2022

Choose a reason for hiding this comment

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

Yes, i also think the mentor path needs a bit more thought!

Copy link
Collaborator

@anthonykhoa anthonykhoa Oct 6, 2022

Choose a reason for hiding this comment

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

The exercises shouldn't be under /exercises but something like /curriculum/js0/exercises.

@flacial Can you explain your reasoning on why the above should mean that we shouldn't make a /exercises constant?

Copy link
Member Author

@flacial flacial Oct 6, 2022

Choose a reason for hiding this comment

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

I thought /exercises/js0 would make things inconsistent with the way we manage things that are related to a lesson (e.g, challenges) and thus it should be changed to something like /curriculum/js0/exercises. But it appears the lesson submissions are under /review/js0.

I'l create one for /exercises in a different PR as this is merged.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought /exercises/js0 would make things inconsistent with the way we manage things that are related to a lesson (e.g, challenges) and thus it should be changed to something like /curriculum/js0/exercises.

I'm not understanding the reasoning -- even if the path is /curriculum/js0/exercises, why do you think a /exercises constant should still not be made for that?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought /exercises/js0 would make things inconsistent with the way we manage things that are related to a lesson (e.g, challenges) and thus it should be changed to something like /curriculum/js0/exercises.

I'm not understanding the reasoning -- even if the path is /curriculum/js0/exercises, why do you think a /exercises constant should still not be made for that?

I decided not to add that constant because, based on the possibility of introducing inconsistency in how we manage things that are related to a lesson, we could change the exercises' paths before launching DOJO. I was wrong as we already have the /review/[lessonSlug] path.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was thinking that since we are going to need /exercises in the path it would be a good idea to make the constant anyway(which I see you note you'll do in another PR). Whether the path is /curriculum/js0/exercises or /exercises/js0 it needs /exercises. The reason I think /exercises should be a constant is because of its high potential for reusability -- if you search the path /curriculum for example, you can find a lot of results of its usage,
https://github.com/garageScript/c0d3-app/search?p=4&q=%2Fcurriculum

Copy link
Member Author

@flacial flacial Oct 6, 2022

Choose a reason for hiding this comment

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

I like this idea. By creating the exercises constant, we can join constants to create paths like the following CURRICULUM_PATH/[lessonSlug]/EXERCISES_PATH.

{
text: 'mentor exercises',
url: `${CURRICULUM_PATH}/${lesson.slug}/mentor`
}
]

return (
<NavCard
tabSelected={tabs.findIndex(tab => tab.text === activeTab)}
tabs={tabs}
/>
)
}

export default LessonTabs
1 change: 1 addition & 0 deletions components/LessonTabs/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from './LessonTabs'
22 changes: 12 additions & 10 deletions pages/curriculum/[lessonSlug]/mentor/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,17 @@ import { GetExercisesQuery, useGetSessionQuery } from '../../../../graphql'
import Error, { StatusCode } from '../../../../components/Error'
import LoadingSpinner from '../../../../components/LoadingSpinner'
import AlertsDisplay from '../../../../components/AlertsDisplay'
import NavCard from '../../../../..../../components/NavCard'
import ExercisePreviewCard from '../../../../..../../components/ExercisePreviewCard'
import { NewButton } from '../../../../..../../components/theme/Button'
import GET_EXERCISES from '../../../../..../../graphql/queries/getExercises'
import styles from '../../../../scss/exercises.module.scss'
import LessonTabs from '../../../../components/LessonTabs'

type ExerciseLesson = {
title: string
docUrl?: string | null
slug: string
}

const MentorPage: React.FC<QueryDataProps<GetExercisesQuery>> = ({
queryData
Expand Down Expand Up @@ -65,7 +71,7 @@ const MentorPage: React.FC<QueryDataProps<GetExercisesQuery>> = ({
tabs={tabs}
lessonTitle={currentLesson.title}
exercises={currentExercises}
lessonSlug={currentLesson.slug}
lesson={currentLesson}
/>

{alerts && <AlertsDisplay alerts={alerts} />}
Expand All @@ -81,24 +87,20 @@ type ExerciseListProps = {
problem: string
answer: string
}[]
lessonSlug: string
lesson: ExerciseLesson
}

const ExerciseList = ({
tabs,
lessonTitle,
exercises,
lessonSlug
lesson
}: ExerciseListProps) => {
const router = useRouter()

return (
<>
<div className="mb-4">
<NavCard
tabSelected={tabs.findIndex(tab => tab.text === 'mentor exercises')}
tabs={tabs}
/>
<LessonTabs lesson={lesson} activeTab="mentor exercises" />
</div>
<div className="d-flex flex-column flex-md-row justify-content-between align-items-center">
<h1 className="my-2 my-md-5 fs-2">{lessonTitle}</h1>
Expand All @@ -108,7 +110,7 @@ const ExerciseList = ({
<NewButton
className="flex-grow-1"
onClick={() =>
router.push(`/curriculum/${lessonSlug}/mentor/addExercise`)
router.push(`/curriculum/${lesson.slug}/mentor/addExercise`)
}
>
ADD EXERCISE
Expand Down
33 changes: 12 additions & 21 deletions pages/exercises/[lessonSlug].tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
import Error, { StatusCode } from '../../components/Error'
import LoadingSpinner from '../../components/LoadingSpinner'
import AlertsDisplay from '../../components/AlertsDisplay'
import NavCard from '../../components/NavCard'
import ExercisePreviewCard, {
ExercisePreviewCardProps
} from '../../components/ExercisePreviewCard'
Expand All @@ -20,6 +19,13 @@ import ExerciseCard, { Message } from '../../components/ExerciseCard'
import { ArrowLeftIcon } from '@primer/octicons-react'
import GET_EXERCISES from '../../graphql/queries/getExercises'
import styles from '../../scss/exercises.module.scss'
import LessonTabs from '../../components/LessonTabs'

type ExerciseLesson = {
title: string
docUrl?: string | null
anthonykhoa marked this conversation as resolved.
Show resolved Hide resolved
slug: string
}

const Exercises: React.FC<QueryDataProps<GetExercisesQuery>> = ({
queryData
Expand Down Expand Up @@ -51,18 +57,6 @@ const Exercises: React.FC<QueryDataProps<GetExercisesQuery>> = ({
if (!currentLesson)
return <Error code={StatusCode.NOT_FOUND} message="Lesson not found" />

const tabs = [
...(currentLesson.docUrl
? [{ text: 'lessons', url: currentLesson.docUrl }]
: []),
{ text: 'challenges', url: `/curriculum/${currentLesson.slug}` },
{ text: 'exercises', url: `/exercises/${currentLesson.slug}` },
{
text: 'mentor exercises',
url: `/curriculum/${currentLesson.slug}/mentor/`
}
]

const currentExercises = exercises
.filter(exercise => exercise?.module.lesson.slug === slug)
.map(exercise => {
Expand Down Expand Up @@ -103,12 +97,12 @@ const Exercises: React.FC<QueryDataProps<GetExercisesQuery>> = ({
/>
) : (
<ExerciseList
tabs={tabs}
onClickSolveExercises={() => setSolvingExercise(true)}
lessonTitle={currentLesson.title}
hideAnswered={hideAnswered}
setHideAnswered={setHideAnswered}
exercises={currentExercises}
lesson={currentLesson}
/>
)}
{alerts && <AlertsDisplay alerts={alerts} />}
Expand Down Expand Up @@ -233,29 +227,26 @@ type ExerciseListItem = {
}

type ExerciseListProps = {
tabs: { text: string; url: string }[]
onClickSolveExercises: () => void
lessonTitle: string
hideAnswered: boolean
setHideAnswered: (hideAnswered: boolean) => void
exercises: ExerciseListItem[]
lesson: ExerciseLesson
}

const ExerciseList = ({
tabs,
onClickSolveExercises,
lessonTitle,
hideAnswered,
setHideAnswered,
exercises
exercises,
lesson
}: ExerciseListProps) => {
return (
<>
<div className="mb-4">
<NavCard
tabSelected={tabs.findIndex(tab => tab.text === 'exercises')}
tabs={tabs}
/>
<LessonTabs lesson={lesson} activeTab={'exercises'} />
flacial marked this conversation as resolved.
Show resolved Hide resolved
</div>
<div className="d-flex flex-column flex-md-row justify-content-between align-items-center">
<div className="my-2 my-md-5">
Expand Down
18 changes: 18 additions & 0 deletions stories/components/LessonTabs.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import React from 'react'
import LessonTabs from '../../components/LessonTabs'

export default {
components: LessonTabs,
title: 'Components/LessonTabs'
}

export const _LessonTabs: React.FC = () => (
<LessonTabs
lesson={{
title: 'Foundations of JavaScript',
docUrl: '',
slug: 'js0'
}}
activeTab={'exercises'}
/>
)