Skip to content
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

Add second level resolution #12

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add second level resolution #12

wants to merge 1 commit into from

Conversation

bhelx
Copy link
Member

@bhelx bhelx commented Jun 4, 2023

Right now, we just say "less than a minute ago" because we were only getting minute level resolution from the server. But we can request second level with the tmres query paramter. So this updates the client to ask for second resolution and shows this to the user. It gives them a little more context on whether they are looking at live data or some kind of ghost vehicle. Instead of showing the exact number of seconds I'm rounding the number of seconds to the nearest 5 so we don't overexercise react.

Before

Screenshot 2023-06-04 at 10 41 56 AM

After

Screenshot 2023-06-04 at 10 41 45 AM

Right now, we just say "less than a minute ago" because we were only
getting minute level resolution from the server. But can request second
level with the `tmres` query paramter. So this updates the client to ask
for second resolution and shows this to the user. It gives them a little
more context on whether they are looking at live data or some kind of
ghost vehicle. Instead of showing the exact number of seconds I'm
rounding the number of seconds to the nearest 5 so we don't overexercise
react.
@chr1sto14
Copy link
Contributor

I found out that the timezone parsing isn't very robust. The seconds division looks good, but the parsing section needs something like the following so timestamp show up correctly no matter the device timezone.

const timestamp = Date.UTC(
	ts.slice(0,4), // year
	ts.slice(4,6)-1, // month (0 based)
	ts.slice(6,8), // day
	ts.slice(9,11), // hour
	ts.slice(12,14), // minute
	ts.slice(15,17), // second
);
const now = new Date()
const utcDate = new Date(now.toLocaleString('en-US', { timeZone: 'UTC' }));
const nolaDate = new Date(now.toLocaleString('en-US', { timeZone: 'America/Chicago' }));
const offset = utcDate.getTime() - nolaDate.getTime();
const relativeTimestamp = now.getTime() - timestamp - offset;

@harveysanders
Copy link
Contributor

Out of curiosity, what's the advantage of moving the time parsing to the client? Performance gains from less marshaling, unmarshaling? Doesn't this come at the cost of losing the Vehicle types and the built-in, battle-tested time parsing in go?
With the time parsed to on the backend, couldn't you just pass the time string directly to the Date constructor and have the timezone sorted? Seems like less bug surface area.

Again, just asking questions to better understand the app to better contribute eventually :)

@chr1sto14
Copy link
Contributor

The Go server pipes the API response data along to the client. All of the business logic is concentrated on the client now. I'm not sure about performance impacts.

@harveysanders
Copy link
Contributor

Gotcha. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants