Feature/note detail page#172
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
TyHil
left a comment
TyHil
left a comment
There was a problem hiding this comment.
Overall this is a great start! Just some things to start changing then hit the rerequest review button when you're done.
Let's use the royal color on light more and cornflower-300 color on dark mode for the accents, icons, and things, to match the rest of the site. And make sure your page works on dark mode too. It's ok to deviate from the design some.
…rthe notes. May change when I figure out how the iFrame works.
… still working on that, but I managed to figure out what was requiring the second reload before the pdf showed up.
…animation is still a bit janky though.
TyHil
left a comment
TyHil
left a comment
There was a problem hiding this comment.
Sweet! I think this is almost there! Just some smaller things I noticed to change before it's done
| import { Collapse, IconButton } from '@mui/material'; | ||
| import Link from 'next/link'; // To link back to specific profiles | ||
| import { useState } from 'react'; | ||
| import NoteEditButton from '@src/components/sections/NoteEditButton'; |
There was a problem hiding this comment.
Could you add the delete and report buttons as well?
| {file.publicUrl ? ( | ||
| {/* Scrollable area with white card background for the PDF */} | ||
| <div className="h-hull w-full max-w-6xl"> | ||
| <div className="mx-auto rounded-2xl overflow-hidden shadow-lg max-w-[1200px] bg-white dark:bg-neutral-800"> |
There was a problem hiding this comment.
Instead of these 2 nested divs over the iframe try:
<BaseCard className="h-hull w-full max-w-6xl">
<iframe
src={file.publicUrl}
title={file.name}
className="w-full h-[80vh] rounded border-0"
/>
</BaseCard>
That way we're using the standardized corner radius and shadow from the BaseCard
|
|
||
| return ( | ||
| <div className="w-full max-w-6xl"> | ||
| <div className="pointer-events-auto bg-white dark:bg-neutral-800 rounded-2xl shadow-lg border-l-4 border-royal dark:border-cornflower-300"> |
There was a problem hiding this comment.
Instead of these 2 outer divs try using <Panel className="w-full pb-2 border-l-4 border-royal dark:border-cornflower-300" smallPadding> for the same reasons as the last comment.
| <div className="flex items-center gap-3 min-w-0 flex-1"> | ||
| <SaveButton fileId={fileId} iconOnly /> | ||
| <h1 className="font-bold text-lg leading-tight truncate text-royal dark:text-cornflower-300"> | ||
| {displayTitle} |
There was a problem hiding this comment.
Instead of having the course in displayTitle can you just use the {name} here. That way we can add the course down next to the author and professor as another hyperlink!
| <div className="w-full max-w-6xl"> | ||
| <div className="pointer-events-auto bg-white dark:bg-neutral-800 rounded-2xl shadow-lg border-l-4 border-royal dark:border-cornflower-300"> | ||
| {/* Top piece, always visible, has the class, section, note name, and rating */} | ||
| <div className="flex items-center justify-between px-5 pt-4 pb-2 gap-4"> |
There was a problem hiding this comment.
I think adding flex-wrap to this div then removing min-w-0 from the div inside and removing truncate from the div inside that will help with mobile
| <div className="w-full max-w-6xl"> | ||
| <div className="pointer-events-auto bg-white dark:bg-neutral-800 rounded-2xl shadow-lg border-l-4 border-royal dark:border-cornflower-300"> | ||
| {/* Top piece, always visible, has the class, section, note name, and rating */} | ||
| <div className="flex items-center justify-between px-5 pt-4 pb-2 gap-4"> |
There was a problem hiding this comment.
Try removing the pt-4 pb-2 here on this div and changing the pb-4 on the next div (inside the Collapse) to a pt-2. This will make the NoteInfoPanel look largely the same but it'll collapse smaller, since the padding is inside the Collapse.
2 participants
Single notes page design issue; Note page has somewhat matching UI to the Figma. It works but I need to fix some of the proportions.