59 view degree course page#75
Conversation
3e1b497 to
cca1985
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “Degree course” page and supporting UI/components, backed by a local JSON dataset and exposed via Storybook.
Changes:
- Added a
DegreeCoursepage that renders course metadata, subjects table, and opinions list fromindexes_majors.json. - Introduced new UI building blocks (Card, Table, ScrollArea, Collapsible, Button, Toggle) and new domain components (
TableOfSubjects,TableOfOpinions,Opinion). - Added a Storybook story for the new page and added a new Radix-related dependency.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/stories/degree_course.stories.ts | Adds Storybook coverage for the new degree course page. |
| client/src/pages/degree_course.tsx | Implements the new page and data loading/rendering. |
| client/src/components/ui/toggle.tsx | Adds a small toggle button component for label switching. |
| client/src/components/ui/table.tsx | Adds a shared table component set. |
| client/src/components/ui/scroll-area.tsx | Adds a scroll container component. |
| client/src/components/ui/collapsible.tsx | Adds a collapsible wrapper component. |
| client/src/components/ui/card.tsx | Adds card layout components. |
| client/src/components/ui/button.tsx | Adds a shared button component. |
| client/src/components/subjects/tableOfSubjects.tsx | Renders per-semester subjects in a table. |
| client/src/components/opinions/tableOfOpinions.tsx | Renders opinions with collapsible “show more”. |
| client/src/components/opinions/opinion.tsx | Renders a single opinion entry. |
| client/src/assets/data/indexes_majors.json | Provides mock degree-course data used by the page. |
| client/package.json | Adds radix-ui dependency. |
| client/bun.lock | Lockfile updates for the new dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { Collapsible as CollapsiblePrimitive } from 'radix-ui'; | ||
|
|
||
| function Collapsible({ ...props }: React.ComponentProps<typeof CollapsiblePrimitive.Root>) { | ||
| return <CollapsiblePrimitive.Root data-slot="collapsible" {...props} />; | ||
| } |
| <ScrollArea className="h-52 rounded-md border p-4"> | ||
| <Table> | ||
| <TableHeader>Przedmioty w kierunku Informatyka </TableHeader> | ||
| <TableHeader> | ||
| <TableRow> | ||
| {semesters.length > 0 ? ( | ||
| semesters.map(function (semester) { | ||
| return <TableHead key={semester.number}>Semestr {semester.number}</TableHead>; | ||
| }) | ||
| ) : ( | ||
| <TableHead>Semestr</TableHead> | ||
| )} | ||
| </TableRow> | ||
| </TableHeader> |
| <TableRow> | ||
| <TableCell colSpan={semesters.length}>Brak danych</TableCell> | ||
| </TableRow> |
| const myJson = await response.json(); | ||
| console.log('Dane pobrane pomyślnie:', myJson); | ||
| if (Array.isArray(myJson)) { |
There was a problem hiding this comment.
Agreed, in PR's there should be none of console.log's
| worst_subjects: DegreeCourseWorstSubject[]; | ||
| opinions: DegreeCourseOpinion[]; // Dodany opcjonalny klucz dla opinii | ||
| } |
| semesters: [], | ||
| absolvent_future: '', | ||
| worst_subjects: [{ name: '', mark: 0 }], | ||
| opinions: [], | ||
| }; |
| <CollapsibleTrigger> | ||
| <Toggle OnValue="Zwiń" OffValue="Rozwiń" /> | ||
| </CollapsibleTrigger> |
| import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card.tsx'; | ||
| import { ScrollArea } from '@/components/ui/scroll-area'; | ||
| import { Collapsible, CollapsibleTrigger, CollapsibleContent } from '@/components/ui/collapsible'; | ||
| import Toggle from '@/components/ui/toggle.tsx'; | ||
| import Opinion from '@/components/opinions/opinion.tsx'; |
| interface semester { | ||
| number: number; | ||
| subjects: string[]; | ||
| } | ||
|
|
||
| export default function TableOfSubjects({ semesters }: { semesters: semester[] }) { | ||
| return ( |
RomanGumeniuk
left a comment
There was a problem hiding this comment.
Looks really good, great job! I also have a few changes to discuss.
I think we should stick with data fetching using TanStack Query + Zod setup from #71, and keeping shared types in one place. Also one little thing can you please add a type:task label?
Overall, well done 🍻
| } finally { | ||
| setLoading(false); | ||
| } | ||
| } |
There was a problem hiding this comment.
In #70 we moved the course page to TanStack Query + a Zod
schema instead of manual useState/useEffect/fetch. Could we align this
page to the same pattern for consistency? 🙏
Here's how it looks in my useCourse hook:
async function fetchCourse(slug: string): Promise<Course> {
const response = await agent.get(`/mocks/${slug}.json`);
return CourseSchema.parse(response.data);
}
export function useCourse(slug: string) {
return useQuery<Course>({
queryKey: ['course', slug],
queryFn: () => fetchCourse(slug),
staleTime: 1000 * 60 * 5,
});
}This way useQuery gives us loading/error/caching without need to write it manually and Zod validates the data and infers the type so we don't need to write interface manually.
Michał shared this video that helped me understand whats going on:
TanStack Query - How to Master God-Tier React Query
| } | ||
| useEffect(function () { | ||
| // Używamy pliku JSON jako domyślnego mocka jeśli jest dostępny, | ||
| // w przeciwnym razie wykonujemy fetch (np. w środowisku produkcyjnym). |
There was a problem hiding this comment.
small one, let's keep code comments in English to stay consistent
across the repo. UI strings in Polish are fine of course, just the code
| setMajor(fallbackMajor); | ||
| } | ||
| } else { | ||
| setMajor(myJson as DegreeCourseData); |
There was a problem hiding this comment.
this line trusts the JSON without checking it. If the data is wrong the app will break later and it will be hard to find the problem. Maybe use a Zod schema here? It checks data at runtime and gives us the type.
Here's how i did it in #70:
// schemas/course.ts
const CourseSchema = z.object({
name: LocalizedStringSchema,
ectsPoints: z.number().nullable(),
// ...
});
type Course = z.infer<typeof CourseSchema>; // type is coming straight from the schema// so when loading data:
return CourseSchema.parse(response.data); // it throws if data is wrong| @@ -0,0 +1,14 @@ | |||
| interface Opinion { | |||
There was a problem hiding this comment.
the Opinion type is written 3 times (opinion,tableOfOpinions and degree_course). Can we keep it in in schemas/ and import it? This way we change it in one place and also other pages will probably reuse it too 🍺
| return ( | ||
| <Card> | ||
| <CardHeader> | ||
| <CardTitle className="text-2xl font-bold" style={{ textAlign: 'center' }}> |
There was a problem hiding this comment.
small thing, for style consistency better to use tailwind here instead of inline style
so instead of style={{ textAlign: 'center' }} it's better to just use tailwinds text-center
Changes
How to test (optional)
bun run storybook
Screenshots / recordings (for UI stuff)
Checklist
To have your PR reviewed put the link e.g.
https://github.com/akai-org/put-wiki/pull/0to theReview PRthread on put-wiki dc channel (you must be member of the AKAI discord server)