chore: alert banner for HashiConf #2584
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
heatlikeheatwave
requested review from
a team,
EnMod and
prestonbourne
and removed request for
a team and
EnMod
📦 Next.js Bundle AnalysisThis analysis was generated by the next.js bundle analysis action 🤖 This PR introduced no changes to the javascript bundle 🙌 |
src/layouts/base-layout/index.tsx
Outdated
| const [isDuringHashiConf, setIsDuringHashiConf] = useState(false) | ||
|
|
||
| useEffect(() => { | ||
| const now = new Date().getTime() // current date & time |
There was a problem hiding this comment.
@heatlikeheatwave we have a timezone bug here. Namely we are getting a random timezone but then comparing it to PST, which could mean they are already in Hashiconf time when Hashiconf hasn't even started yet. We need to get the users timezone in PST to accurately compare to PST.
So now should be:
const now = new Date().toLocaleString('en-US', { timeZone: 'America/Los_Angeles' })
There was a problem hiding this comment.
Actually that is just going to just return the string. I think we should import something like: https://day.js.org/en/ and use it's date manipulation much easier.
As we need to:
- Get the users date
- Set the users date timezone to PST
- Compare that date between two different PST timezones
There was a problem hiding this comment.
Shouldn't it be EDT since the conference is in Boston?
There was a problem hiding this comment.
Also, great catch with this!
There was a problem hiding this comment.
Dates. Why you so hard. 😢
There was a problem hiding this comment.
Y'all are making it happen with more 👁️ on the code.
There was a problem hiding this comment.
@RubenSandwich I tested this again, and I don't think this has the bug you described. I changed my location and time on my machine to west coast and europe; the visibility of the banner worked as expected. What am I missing that you're seeing?
I do agree that the pacific time zone (.e.g -8:00 in the time stamp) can be changed. That was a mistake on my part.
I think by converting the dates to unix timestamps that removes any confusion about timezones, no?
heatlikeheatwave
changed the title
src/layouts/base-layout/index.tsx
Outdated
| @@ -6,7 +6,7 @@ | |||
| // Third-party imports | |||
| import classNames from 'classnames' | |||
| import AlertBanner from '@hashicorp/react-alert-banner' | |||
| import { HTMLAttributes, useState } from 'react' | |||
| import { HTMLAttributes, useEffect, useState } from 'react' | |||
There was a problem hiding this comment.
I don't think the useEffect is used? But as long as the linter doesn't complain and it builds I have no issue with it.
🔗 Relevant links
🗒️ What
📸 Design Screenshots
🧪 Testing