Skip to content
This repository has been archived by the owner on Aug 26, 2023. It is now read-only.

Some times don't respect local format #292

Open
WhyNotHugo opened this issue Apr 29, 2022 · 16 comments · May be fixed by #327
Open

Some times don't respect local format #292

WhyNotHugo opened this issue Apr 29, 2022 · 16 comments · May be fixed by #327
Assignees
@triallax
Labels
bug Something isn't working UI

Comments

@WhyNotHugo
Copy link
Contributor

Describe the bug

I use 24hs time format, e.g.: without subtracting 12 after midday.

In a few spots, the application doesn't respect this and uses the am/pm time format.

Times that respect system locale

  • Sunrise
  • Sunset

Times that DO NOT respect system locale

  • Last updated time on the top
  • Times for today's forecast (e.g.: In the middle of the main screen).

This last one is the really annoying one, since it makes reading the forecast a bit trickier.

Screenshots

image

image

image

Smartphone (please complete the following information):

  • Device: OnePlus 9
  • OS: LineageOS 18
  • App version: 2.0.1

Additional context

Thanks for the very beautiful and simple weather app! 🍻
@WhyNotHugo WhyNotHugo added the bug Something isn't working label Apr 29, 2022
@triallax triallax added the UI label Apr 29, 2022
@triallax
Copy link
Member

This should be pretty easy to fix; I might ping you on the PR that fixes this so that you can take a look.

@triallax triallax self-assigned this Apr 29, 2022
@triallax triallax added this to the v2.0.2 milestone Apr 29, 2022
@WhyNotHugo
Copy link
Contributor Author

The ones that are correct use DateFormat.Hm().format:

clima/packages/clima_ui/lib/widgets/weather/additional_info_widget.dart

Lines 110 to 112 in a652d03

value: DateFormat.Hm().format(
currentDayForecast.sunrise.toUtc().add(timeZoneOffset),
),

These don't:

clima/packages/clima_ui/lib/screens/weather_screen.dart

Line 132 in c646836

'Updated ${DateFormat.Md().add_jm().format(fullWeather.currentWeather.date.toLocal())} · ',

clima/packages/clima_ui/lib/widgets/weather/hourly_forecasts_widget.dart

Lines 46 to 48 in c646836

DateFormat.j().format(
hourlyForecast.date.toUtc().add(timeZoneOffset),
),

@WhyNotHugo
Copy link
Contributor Author

WhyNotHugo commented Apr 29, 2022 •
edited
Loading

Huh, dart is pretty easy to read... At least for these simple UI bits.

@triallax
Copy link
Member

Something is weird; j is supposed to follow the system locale, while H forces the 24-hour format (see https://api.flutter.dev/flutter/intl/DateFormat-class.html). Why isn't j following the system locale as it's supposed to?

@WhyNotHugo
Copy link
Contributor Author

Yup, reading those docs it would seem that using j for all these instances should be the correct approach.

But the places that do use j are rendering in US format for me... Is the OS locale automatically picked up? Maybe initializeDateFormatting needs to be called with the system setting...?

@WhyNotHugo
Copy link
Contributor Author

Never mind:

If the optional locale is omitted, the format will be created using the default locale in Intl.systemLocale.

@triallax
Copy link
Member

From https://api.flutter.dev/flutter/intl/Intl/systemLocale.html:

Note that due to system limitations this is not automatically set, and must be set by importing one of intl_browser.dart or intl_standalone.dart and calling findSystemLocale().

Maybe this is it?

@WhyNotHugo
Copy link
Contributor Author

Oh, nice find!

@WhyNotHugo
Copy link
Contributor Author

So I guess it's a matter of calling that at startup, and the using j in

clima/packages/clima_ui/lib/widgets/weather/additional_info_widget.dart

Lines 110 to 112 in a652d03

value: DateFormat.Hm().format(
currentDayForecast.sunrise.toUtc().add(timeZoneOffset),
),

@triallax
Copy link
Member

Yeah, that sems like what we should do. I'm wondering now though about how to handle the system locale changing while the app is running.

@WhyNotHugo
Copy link
Contributor Author

WhyNotHugo commented Apr 29, 2022 •
edited
Loading

It doesn't seem the abstractions provided account for that ever happening.

Edit: Maybe call findSystemLocale when the app re-gains focus?

@triallax
Copy link
Member

Maybe, but I guess we can leave that to another PR?

@WhyNotHugo
Copy link
Contributor Author

Yeah, that seems like another issue IMHO.

@WhyNotHugo

This comment was marked as off-topic.

Sign in to view
@triallax

This comment was marked as off-topic.

Sign in to view
@WhyNotHugo WhyNotHugo mentioned this issue May 3, 2022
Open
@triallax
Copy link
Member

triallax commented Jun 7, 2022

I tried to fix this yesterday, and it turned out to be more of a pain than I had expected, though it's mostly done. Due to locale stuff, fixing #91 first is probably a good idea. In any case, I'm going to submit a draft PR with my work on this issue.

@triallax triallax removed this from the v2.0.2 milestone Jun 7, 2022
triallax added a commit to triallax/clima that referenced this issue Jun 28, 2022
@triallax
[ui] follow system time format
e6c31d8 
Fixes Lacerte#292.
@triallax triallax linked a pull request Jun 28, 2022 that will close this issue
Draft
2 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@triallax triallax

Labels
bug Something isn't working UI
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[ui] follow system time format triallax/clima
2 participants
@WhyNotHugo @triallax