-
Notifications
You must be signed in to change notification settings - Fork 2
Feat/schedule page #191
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
Feat/schedule page #191
Conversation
|
|
||
| type EventType = 'GENERAL' | 'ACTIVITIES' | 'WORKSHOP' | 'MENU'; | ||
|
|
||
| interface CalendarItemProps { |
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 the Event type defined in app/_types/event.ts instead for consistency across the codebase and backward compatibility. if the latest changes in the wireframes requires the DB schema to change, notify the team on Slack.
| type: EventType; | ||
| startTime: string; | ||
| endTime: string; | ||
| location?: string; |
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.
don't think location will be optional. We will probably display some sort of location (like "Hacker Floor" or "East Wing") for every event
| import CalendarItem from '../_components/HackerHub/CalendarItem'; | ||
| type EventType = 'GENERAL' | 'ACTIVITIES' | 'WORKSHOP' | 'MENU'; | ||
|
|
||
| interface Event { |
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 here, use the type defined in _types
| startTime: string; | ||
| endTime: string; | ||
| location?: string; | ||
| speakers?: { |
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.
don't think we'll display a speaker name. it'll just be company name. in the db schema, this field is called "host"
|
|
||
| const formatTime = (timeStr: string): string => { | ||
| const date = new Date(timeStr); | ||
| // Convert to PST |
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.
shouldn't need to convert to PST. we'll input PST strings in the DB. I think this conversion is actually rendering the wrong time in the schedule (eg, 1:30am instead of 8:30am for the first event in your mockScheduleData)
| [key: string]: Event[]; | ||
| } | ||
|
|
||
| const mockScheduleData: ScheduleData = { |
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.
default DB response will have a different structure. Wont have the keys "Apr19" and "Apr20", it'll just be an array of event documents. You'll need to manipulate it to get this structure.
| return ( | ||
| <main> | ||
| <div>Schedule Page</div> | ||
| <main className="w-screen px-[80px] mt-[60px]"> |
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.
never use w-screen. each browser has a different definition of viewport width, some include the scrollbar's width and some don't leading to different, and sometimes annoying, behavior like needing a horizontal scroll bar to see that additional tiny scrollbar width on the right. i've fixed it in the commit i made as this was more urgent.
No description provided.