Add popover to indicate the current meal block period #712
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
oh yea also I have like three midterms next week 😭 so feel free to approve and merge other ppl's PRs without my approval. I think main thing is just make sure the class names are named well and that css modules are used whenever possible. |
|
試験がんばってね、エリック！ |
|
I like how you bold the active row here! thoughts on this revision of your design? (filter has been moved to the right, closer to the search bar, which makes more sense I think) I also added a button so ppl on mobile can click on it (actually we can probably make the whole row clickable and hoverable) 
|
src/pages/ListPage.tsx
Outdated
| const now = new Date(); | ||
| const pittsburghTime = new Date(now.toLocaleString('en-US', { timeZone: 'America/New_York' })); | ||
| const seconds = pittsburghTime.getHours() * 3600 + pittsburghTime.getMinutes() * 60 + pittsburghTime.getSeconds(); |
There was a problem hiding this comment.
I'm 90% sure this doesn't do what you think it does - this is probably equivalent to const pittsbughTime = new Date() (and new Date().getHours() defaults to whatever timezone your computer is set to)
You can use the luxon package's DateTime.now().setZone('America/New_York') instead to force the timezone to be in pittsburgh time, no matter what the computer's local timezone is
There was a problem hiding this comment.
I made this in MN (cst) and it worked out fine then! but i'll test a bit more
src/pages/ListPage.tsx
Outdated
| const shouldAnimateCards = useRef(true); | ||
|
|
||
| const blockPeriods = getBlockPeriodsWithTimes(); | ||
| const currentPeriod = getBlockPeriod(); |
There was a problem hiding this comment.
might be good to make getBlockPeriod a pure function that takes in the current time instead. (helps with testing and re-rendering if we decide to periodically update what "now" represents)
src/pages/ListPage.tsx
Outdated
| { period: 'Dinner', timeRange: '04:30 PM - 08:59 PM' }, | ||
| { period: 'Late Night', timeRange: '09:00 PM - 03:29 AM' }, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
any reason why this is decoupled from
const breakfastStart = 3 * 3600 + 30 * 60;
const lunchStart = 10 * 3600 + 30 * 60;
const dinnerStart = 16 * 3600 + 30 * 60;
const lateNightStart = 21 * 3600;
src/pages/ListPage.tsx
Outdated
| <div onMouseEnter={handleMouseEnter} onMouseLeave={handleMouseLeave} className="block-period"> | ||
| <p className={`block-period__current ${isPopupVisible ? 'block-period__current--hidden' : ''}`}> |
There was a problem hiding this comment.
might be good to extract all of this into a separate component/file (its functionality is completely separate from everything else)
|
I like the buttons you added i think it could be helpful for mobile! |
|
What specifically about this feels off? 
and yea maybe we can pick a better font but I think having a monospaced one helps with readability or ig this (Inter) is fine How about de-emphasizing the text a bit for mobile? or maybe two lines? |
|
or this? |
I prefer the popup under the searchbar, and I don't really like how it disrupts the margin and comes in from the right. But I like your idea to have a different font, I like the monospaced font. |
I like this! I just think the header should be the first thing people see and the block periods popup should be with the rest of the filters and I would make it the dark gray color of the rest of the filtering and search stuff |
you don't think that making it the same color as the filtering and search might cause confusion? cuz it's not really part of the search functionality lol |
no lol I think it's all part of the side functionality and not the main eatery card functionality. Also, If its on the other side there's some space between the block period popover and the other filters. Maybe we can switch the sides of the filters to group all of the filters together, but I still think the popover should follow the same color scheme and styling as the filtering stuff. we can poll slack? |
sure |
#706
based on feedback form request