56 implement felvételi tájékoztató page#66
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
.../[lang]/kollegium/felveteli-tajekoztato/components/DormitoryAdmissionIinformationContent.tsx
Outdated
Show resolved
Hide resolved
.../[lang]/kollegium/felveteli-tajekoztato/components/DormitoryAdmissionIinformationContent.tsx
Outdated
Show resolved
Hide resolved
.../[lang]/kollegium/felveteli-tajekoztato/components/DormitoryAdmissionIinformationContent.tsx
Outdated
Show resolved
Hide resolved
peterlipt
left a comment
peterlipt
left a comment
There was a problem hiding this comment.
Requested some changes, but good work still!
| <Suspense fallback={<LoadingRegulationsGrid />}> | ||
| <DormitoryAdmissionInformationContent content={content}/> | ||
| </Suspense> |
There was a problem hiding this comment.
The Suspense component is unnecessary here because the data is already fetched and awaited in the parent server component, so the child component receives the full content immediately as props and never needs to trigger a loading state.
There was a problem hiding this comment.
Each subsection should have its own card. By subsections I mean these: Férőhelyek, Jelentkezés menete, Elérhetőségek;
There was a problem hiding this comment.
The file currently contains multiple components/functions. Extract DormitoryCardsContainer, DormitoryCard, StudentUnionCardsContainer, and StudentUnionCard into their own separate files.
There was a problem hiding this comment.
DormitoryCard and StudentUnionCard are almost identical. Refactor them into a single, reusable InfoCard or ImageCard component.
There was a problem hiding this comment.
Remove the typo from the filename. (double i)
|
|
||
| )} | ||
|
|
||
| export function DormitoryCardsComtainer(){ |
There was a problem hiding this comment.
Fix the typo.
| export function DormitoryCardsComtainer(){ | |
| export function DormitoryCardsContainer(){ |
| {admission_information.capacity_p1} | ||
| </p> | ||
| <div> | ||
| {DormitoryCardsComtainer()} |
There was a problem hiding this comment.
Replace direct function calls (e.g., {DormitoryCardsContainer(...)}) with proper React JSX syntax (e.g., <DormitoryCardsContainer ... />).
| </p> | ||
| <p className="text-gray-700 text-lg richtext text-justify"> | ||
| {admission_information.application_p2} | ||
| <a href="http://kefir.bme.hu" target="_blank" rel="noopener noreferrer" className="font-bold text-lg leading-tight text-gray-900 hover:text-[#862633] transition-colors inline"> |
There was a problem hiding this comment.
Avoid hardcoding specific hex colors. You can use the ehk-dark-red Tailwind class for this color.
| <a href="http://kefir.bme.hu" target="_blank" rel="noopener noreferrer" className="font-bold text-lg leading-tight text-gray-900 hover:text-[#862633] transition-colors inline"> | |
| <a href="http://kefir.bme.hu" target="_blank" rel="noopener noreferrer" className="font-bold text-lg leading-tight text-gray-900 hover:text-ehk-dark-red transition-colors inline"> |
| export function StudentUnionCardsComtainer({ content }: Readonly<{ content: DormitoryAdmissionInformationContent }>){ | ||
| const faculties = content.faculties; | ||
| return ( | ||
| <div className="display-block sm:flex sm:flex-col sm:flex-row flex-wrap items-center justify-center gap-4 md:gap-0 flex-1" > |
There was a problem hiding this comment.
In Tailwind CSS, the correct class for display: block is simply block.
You have applied both sm:flex-col (vertical layout) and sm:flex-row (horizontal layout) to the same element at the same breakpoint (sm). This is contradictory, as an element cannot be both a row and a column at the same time. Determine the intended layout behavior.
|
Thanks for both of the reviews! I fixed all the things you had requested (at least I think so). I have created the PageBlock component for future use as well, because it seems to me that basically every subpage contains something similar. |
3 participants
No description provided.