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.

Sign up for GitHub

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
merged 14 commits into from
Oct 6, 2022
Merged

feat(components): Create LessonTabs #2394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2610195
feat: Create LessonTabs component
flacial Oct 3, 2022
6bcfd85
refactor: Update references of old implementation
flacial Oct 3, 2022
6accfc5
test: Update snapshots
flacial Oct 3, 2022
8595730
refactor: Use constant
flacial Oct 3, 2022
76c7262
test: Update snapshots]
flacial Oct 3, 2022
bd5ffb6
refactor: Use constant
flacial Oct 4, 2022
2a5df87
test: Update snapshot
flacial Oct 4, 2022
d9e1efe
Merge branch 'master' into 2029-dojomentorpage-list-mentor-submitted-…
flacial Oct 4, 2022
c712724
refactor: Type activeTab prop
flacial Oct 5, 2022
0dd1d4d
Merge branch '2029-dojomentorpage-list-mentor-submitted-exercises/5' …
flacial Oct 5, 2022
db7266c
refactor: add ActiveTab enum
flacial Oct 5, 2022
a78c25e
refactor: Update references
flacial Oct 5, 2022
0cd3e52
Merge branch 'master' into 2029-dojomentorpage-list-mentor-submitted-…
flacial Oct 5, 2022
ba48999
Merge branch 'master' into 2029-dojomentorpage-list-mentor-submitted-…
flacial Oct 6, 2022
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
38 changes: 38 additions & 0 deletions __tests__/__snapshots__/storyshots.test.js.snap
View file
Open in desktop
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
View file
Open in desktop
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
42 changes: 42 additions & 0 deletions components/LessonTabs/LessonTabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import React from 'react'
import { CURRICULUM_PATH } from '../../constants'
import NavCard from '../NavCard'

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

export enum LessonTab {
LESSON = 'lesson',
CHALLENGES = 'challenges',
EXERCISES = 'exercises',
MENTOR_EXERCISES = 'mentor exercises'
}

type Props = {
lesson: Lesson
activeTab: LessonTab
}

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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from './LessonTabs'
23 changes: 13 additions & 10 deletions pages/curriculum/[lessonSlug]/mentor/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,18 @@ 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'
import { LessonTab } from '../../../../components/LessonTabs/LessonTabs'

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

const MentorPage: React.FC<QueryDataProps<GetExercisesQuery>> = ({
queryData
Expand Down Expand Up @@ -65,7 +72,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 +88,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={LessonTab.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 +111,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
34 changes: 13 additions & 21 deletions pages/exercises/[lessonSlug].tsx
View file
Open in desktop
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,14 @@ 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'
import { LessonTab } from '../../components/LessonTabs/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 +58,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 +98,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 +228,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={LessonTab.EXERCISES} />
</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
19 changes: 19 additions & 0 deletions stories/components/LessonTabs.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import React from 'react'
import LessonTabs from '../../components/LessonTabs'
import { LessonTab } from '../../components/LessonTabs/LessonTabs'

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

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