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 time since last weather update to weather app #2242

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JustScott
Copy link
Contributor

@JustScott JustScott commented Feb 3, 2025

Provides the time since the last weather update, incrementing the duration from seconds to minutes at 60 seconds, and minutes to hours at 3600 seconds. I'm open to suggestions on changes to the texts position and/or color. This should resolve #2140.

Also, on the simulator the seconds old count is only updated when you open the app, while on hardware the seconds count up live while in the app... I'm not sure why this is.

weather_0s_old
weather_32s_old
weather_1m_old

@JustScott JustScott force-pushed the weather_time_since_last_update branch from bbb52e9 to 9d13f7a Compare February 3, 2025 18:34
Copy link

github-actions bot commented Feb 3, 2025

Build size and comparison to main:

Section Size Difference
text 373296B 288B
data 948B 0B
bss 22536B 0B

Run in InfiniEmu


int64_t secondsSinceEpoch = dateTimeController.CurrentDateTime().time_since_epoch().count() / 1000000000;
int64_t secondsSinceWeatherUpdate = secondsSinceEpoch - optCurrentWeather->timestamp;
if (secondsSinceWeatherUpdate < 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need the 0 check? Unless the number of seconds somehow becomes negative, it shouldn't be necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did have it showing -1 a few times in the simulator, so figured I would add that just to cover it in the off chance it happened on the watch.

@liamcharger
Copy link
Contributor

Personally, I would prefer to see the minutes instead of the seconds when I have recently updated my weather. For instance, instead of displaying “32 seconds old,” I would like to see “Now” and only show the number of minutes after one has passed.

Another aspect that feels more natural to me is using the phrase “X minutes ago” instead of “X minutes old.”

@JustScott
Copy link
Contributor Author

Personally, I would prefer to see the minutes instead of the seconds when I have recently updated my weather. For instance, instead of displaying “32 seconds old,” I would like to see “Now” and only show the number of minutes after one has passed.

Another aspect that feels more natural to me is using the phrase “X minutes ago” instead of “X minutes old.”

I agree with you on using ago instead of old. As for using Now instead of displaying the seconds, I feel this looks a little akward and personally prefer the seconds count, I'd like to hear what others think about this.

now_instead_of_seconds
seconds_instead_of_now

Have to move the label up -1px to prevent the g from being cut off like in the picture above
move_update_text_up

And I move the temp up as well to prevent crowding:
move_all_up

@tituscmd
Copy link
Contributor

tituscmd commented Feb 6, 2025

I like Now as well, but also agree that it looks a bit awkward.
I have some propositions as to how this could be perhaps made better:

Updated now / Updated 12s ago
Fetched now / Fetched 12s ago
Refreshed now / Refreshed 12s ago

Adding another word results in a less awkward look, as well as being more direct and intuitive to understand.

@tituscmd
Copy link
Contributor

tituscmd commented Feb 6, 2025

seconds_instead_of_now

test

In these two images the label doesn't seem to be centered. Is that just me?

@JustScott
Copy link
Contributor Author

JustScott commented Feb 6, 2025

@tituscmd I fixed the centering issue by dynamically positioning the label depending on weather there is one or two digits. I just tried out a bunch of examples of how this could look, everyone should let me know which they prefer!

With a seconds counter (with dynamic alignment)

InfiniSim_2025-02-06_170016
InfiniSim_2025-02-06_170034

Two version of up to date

InfiniSim_2025-02-06_170114
up_to_date

variations of "updated now"

now_instead_of_seconds
InfiniSim_2025-02-06_170643
InfiniSim_2025-02-06_170704
InfiniSim_2025-02-06_170812

"Updated:" with dynamic alignment

InfiniSim_2025-02-06_171040
InfiniSim_2025-02-06_171427

I can make more examples if anyones curious. Just let me know. I personally prefer to just include the seconds count, as it may be useful to some and keeps the label consistent... also the live second count on hardware makes the app feel more lively.

@mark9064 mark9064 added the enhancement Enhancement to an existing app/feature label Feb 7, 2025
@tituscmd
Copy link
Contributor

tituscmd commented Feb 7, 2025

InfiniSim_2025-02-06_170034

In this image, if you look at the weather icon up top it still doesn't look centered. I would suggest aligning the label with a pointer to the weather icon or the day in the middle of the table, those always seem to be centered.

I may be a bit of a perfectionist ;D

@tituscmd
Copy link
Contributor

tituscmd commented Feb 7, 2025

@JustScott Thank you so much for making all these options!

I think after looking at everything, I like Now and 58s ago / 53m ago / 15h ago the most. The others seem to make the app look too crowded.

@JustScott
Copy link
Contributor Author

In this image, if you look at the weather icon up top it still doesn't look centered. I would suggest aligning the label with a pointer to the weather icon or the day in the middle of the table, those always seem to be centered.

I may be a bit of a perfectionist ;D

Ah I see what you mean about it not looking centered, I was just trying to keep the space between the 1m and ago above the second letter of whatever the third day is, so it doesn't look like the label is moving when it moves from 9m to 10m.

@JustScott
Copy link
Contributor Author

@JustScott Thank you so much for making all these options!

I think after looking at everything, I like Now and 58s ago / 53m ago / 15h ago the most. The others seem to make the app look too crowded.

I agree with you on this, but am curious as to how long you think the app should displayNow before switching to Xs ago?

@tituscmd
Copy link
Contributor

tituscmd commented Feb 7, 2025

In this image, if you look at the weather icon up top it still doesn't look centered. I would suggest aligning the label with a pointer to the weather icon or the day in the middle of the table, those always seem to be centered.
I may be a bit of a perfectionist ;D

Ah I see what you mean about it not looking centered, I was just trying to keep the space between the 1m and ago above the second letter of whatever the third day is, so it doesn't look like the label is moving when it moves from 9m to 10m.

I see, that makes sense.
I'd prefer it centered, but I like the idea of making it look like the label isn't moving! :)

@tituscmd
Copy link
Contributor

tituscmd commented Feb 7, 2025

I agree with you on this, but am curious as to how long you think the app should displayNow before switching to Xs ago?

Oh, right. What about at 30s ago?
Or maybe 31s ago, so that the first 30 seconds are displayed as "Now" and the next 30 seconds are displayed as Xs ago

@JustScott
Copy link
Contributor Author

I agree with you on this, but am curious as to how long you think the app should displayNow before switching to Xs ago?

Oh, right. What about at 30s ago? Or maybe 31s ago, so that the first 30 seconds are displayed as "Now" and the next 30 seconds are displayed as Xs ago

Okay, just so anyone reading this is caught up, here's what it'll look like:

now
31s_ago

I'd prefer it centered, but I like the idea of making it look like the label isn't moving! :)

I personally prefer it to be centered over the third words second letter, so I say we see what the others think. I'm open to changing it if people agree it doesn't look centered.
one_minute

Use "Now" for the label until 31 seconds, then display the live time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to an existing app/feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Weather app needs to show time weather was acquired
4 participants