-
Notifications
You must be signed in to change notification settings - Fork 18
478 club page redesign #492
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
base: develop
Are you sure you want to change the base?
Conversation
Also ContactButtons will now only render 1 button
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Except the OfficerList since it doesn't play well with the dynamic truncation
|
Need to do the same for events |
TyHil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking super nice!
| <> | ||
| <Header /> | ||
| <main className="mb-5 flex flex-col space-y-4 p-4"> | ||
| <main className="mb-5 flex flex-col space-y-4 p-4 px-[10vw]"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other pages have been using max-w-6xl. If that still works it would be more consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would probably be better on mobile too
| }); | ||
| const upcomingEvents = events.filter((e) => e.endTime >= now); | ||
| const lastEventDate = | ||
| events.filter((e) => e.startTime <= now).reverse()[0]?.endTime ?? null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be more clean to put lastEventDate in getDirectoryInfo and still use currentTime: now for filtering.
| return ( | ||
| <section | ||
| id="club-body" | ||
| className="w-full rounded-lg grid grid-cols-1 md:grid-cols-5 gap-5 items-start mt-8" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| export default function ClubContactCard({ club }: ClubContactCardProps) { | ||
| return ( | ||
| <BaseCard className="flex flex-col bg-neutral-50 border-slate-200 shadow-sm p-5 gap-2"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove border-slate-200
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with the other cards
| target="_blank" | ||
| className="inline-block w-full" | ||
| > | ||
| <IconButton |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a MUI Button with startIcon
| {club.tags && ( | ||
| <div className="flex flex-wrap gap-1 mt-2"> | ||
| {club.tags.map((tag) => ( | ||
| <Chip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this chip design, should we standardize it to a component and use it in the rest of the code base?
| export default function OfficerList({ | ||
| officers, | ||
| }: { | ||
| officers: NonNullable<RouterOutputs['club']['getDirectoryInfo']>['officers']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SelectOfficer again if that works
|
|
||
| const contentRef = useRef<HTMLDivElement>(null); | ||
|
|
||
| useLayoutEffect(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works great on desktop but on mobile it defaults to showing a lot of officers.
| {needsTruncation && ( | ||
| <div className="mt-2 pt-2 border-t border-slate-300 z-10"> | ||
| <button | ||
| onClick={() => setIsExpanded(!isExpanded)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this have the same styling as the ExpandableMarkdownText Show more button or vice versa? Just in terms of underline/hover color
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move these changes to a separate PR and do the same to the ClubCard there?

Implemented this design by @Isoscelestial: https://www.figma.com/design/snmEoRUgfjAZYSc6hqySkY/Wireframes-for-Jupiter?node-id=1356-3146&p=f&t=BIcwF2agOk5VNxpI-0
getDirectoryInfoinsrc/server/api/routers/club.tsandsrc/server/api/routers/admin.tsfiles to also return the number of members in the club