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

Please respect region time format on new tab #1308

Closed
felixakiragreen opened this issue Sep 26, 2018 · 31 comments · Fixed by brave/brave-core#3256
Closed

Please respect region time format on new tab #1308

felixakiragreen opened this issue Sep 26, 2018 · 31 comments · Fixed by brave/brave-core#3256

Comments

@felixakiragreen
Copy link

felixakiragreen commented Sep 26, 2018

Test plan

See brave/brave-core#3256

Description

New Tab dashboard shows time in AM/PM, even though system settings are set to 24h time.

No similar issues in brave-browser.
Similar issues in brave-laptop:

Steps to Reproduce

  1. Change system locale to some locale that defaults to 24 time, such as Brazilian Portuguese (Mac: System Preferences > Language & Region > Preferred languages > click the plus button)
  2. Open Brave with New Tab

Actual result:

screen shot 2018-09-26 at 17 51 20

Expected result:

Time should match locale time (24-hour if locale defaults to it, such as pt-BR suggested in the test plan above). Switching back to locales that default to 12-hour (such as en-US) should update accordingly.

Reproduces how often:

Always

Brave version (chrome://version info)

Brave | 0.55.6 Chromium: 70.0.3538.16 (Official Build) dev (64-bit)
Revision | 16ed95b41bb05e565b11fb66ac33c660b721f778-refs/branch-heads/3538@{#306}
OS | Mac OS X
JavaScript | V8 7.0.276.9

Reproducible on current release:

Yes, reproducible also on browser-laptop.

Additional Information

After digging through the source code here:
https://github.com/brave/brave-core/blob/master/components/brave_new_tab_ui/components/app.tsx
I can see that brave-ui/old/clock is being used:
https://github.com/brave/brave-ui/blob/master/src/old/clock/index.tsx
Which renders AM/PM time directly.

@felixakiragreen felixakiragreen changed the title Please respect region time format on dashboard Please respect region time format on new tab Sep 26, 2018
@bsclifton bsclifton added this to the 2.x Backlog milestone Sep 26, 2018
@bbondy bbondy modified the milestones: 2.x Backlog, 1.x Backlog Sep 30, 2018
@rebron rebron added browser-laptop-parity priority/P5 Not scheduled. Don't anticipate work on this any time soon. labels Oct 5, 2018
@kartik-budhiraja
Copy link

I would like to work on this issue if it's available.

@srirambv
Copy link
Contributor

@kartik-budhiraja that would be awesome. I've self assigned the issue so that no one else works on it and then submit a PR with the fix.

@srirambv srirambv self-assigned this Jan 18, 2019
@rebron rebron modified the milestone: 1.x Backlog Feb 7, 2019
@alexandersjosten
Copy link

Is anyone still working on this? Otherwise I would like to give it a go.

@kartik-budhiraja
Copy link

I planned to work on it, but unfortunately, cannot any soon, so you are good to go from my side.

@alexandersjosten
Copy link

Alright, great. I'll have a look then.

@geekrumper
Copy link

geekrumper commented Mar 21, 2019

I've took a look at this. Sorry I've never developed React and have no idea how you would put the solutions structurally together. Therefore no pull request.

It seems you're using Intl.React to solve internalization.
This code line shows me that a format is being provided but with an empty array instead of a locale or an array of locales. I do not know if this is being loaded dynamically afterwords or not.

Looking at the examples from Intl.React, a locale should be provided or else it will always fall back to default.

The counter part to this is how locales are being loaded for that package. The following documentation gives me a hint, but I think something in that regard has already been done in this project.

One other thing to note here is that system locale is preferred, because my browser is in english but my system locale is something else.

2019-03-21 14_31_19-

@alexandersjosten
Copy link

Yes, this is pretty much what I've discovered as well -- the issue seem to be in the brave-ui code.
I'll look around a bit to see if the locale can be loaded dynamically afterwards.

@mladenko
Copy link

I don't know if this is still the problem, but if you wont to change 12h to 24h format, you can change to your local language in settings then put it on top, Brave will change time format. I guess it has something to do with navigator.language option.

@alexandersjosten
Copy link

@mladenko Yes, but that would go against the original issue, where the user shouldn't have to change the browser language. At least for my case when trying it, I had to change the language from English to my mother tongue for the clock change to happen, perhaps I did something wrong?

On a side-note, I think I have a working solution here, so I will hopefully make a PR soon (depending on how the PR for brave-ui goes).

@mrfnasse
Copy link

mrfnasse commented May 2, 2019

I managed to solve it by using "English United Kingdom" as language. It is better than having the 12 hour clock format.

If it is impossible to make it work by reading the locale, why not just add one of those "switches" in the settings page, where you can choose 12 or 24 hour manually?

And if not even that is possible, why not make 24 hour clock standard? There are only a few countries in the world where microsoft standard is 12 hour.
43dwH

@Clemenzah
Copy link

Clemenzah commented May 8, 2019

@mrfnasse

I changed my language to English United Kingdom but still I have 12h format clock on new page. What else did you change except this?

Thanks

@mrfnasse
Copy link

mrfnasse commented May 8, 2019

@Clemenzah

Make sure you put it on top, and that you click on those three dots to also use it for Brave user interface. I dont know if both these actions are necessary, but I did it and it works.

image

@bsclifton bsclifton removed the priority/P5 Not scheduled. Don't anticipate work on this any time soon. label May 18, 2019
@bsclifton
Copy link
Member

bsclifton commented Aug 20, 2019

@spoike that option does exist in Nightly and will be working its way to the release channel 😄

Looks like this:
Screen Shot 2019-08-20 at 9 30 46 AM

Also available in the bottom right (on Dev and Nightly I believe):
Screen Shot 2019-08-20 at 9 31 52 AM

@cezaraugusto
Copy link
Contributor

Thanks all for the multiple ideas here.

While it's possible to get the time format based on system locale, in my investigation, it's impossible, at least via web APIs, to detect the time format alone, which means that if your system is using a locale that defaults to 24-hour, then our NTP will display the 24-hour clock. i.e., if your system defaults to en-US, switching to the 24-hour system will take no effect. Same applies for the opposite, changing to pt-BR (Brazilian Portuguese) will always display a 24-hour clock.

We do have plans to allow users to customize the clock, but this is a matter for a different issue. As my comment here, I'm going to update this issue test plan so we can have a better approach to time than what we have right now, otherwise we would need to label this as wontfix and close the issue.

cezaraugusto added a commit to brave/brave-core that referenced this issue Aug 29, 2019
fix brave/brave-browser#1308

this change allows users with 24-hour format to see time
on NTP the same way their system defined. defaults to whichever
locale user defined on their system settings.
@bsclifton bsclifton added this to the 0.71.x - Nightly milestone Sep 3, 2019
@DaPiGH
Copy link

DaPiGH commented Sep 6, 2019

LOOKING GOOD.
A small trap: "English", which seems to be the default, gives the same result as "English (United States)" you need (possibly) to add "English (United Kingdom)" as a language and select that to get 24h format.
Thanks guys.

@drasbeck
Copy link

Is this implemented in Version 0.68.140 Chromium: 77.0.3865.90 (Official Build) (64-bit)?

I am still looking at a mix of 24h and 12h here.

image

I'm using Denmark as region and English (US) as language on macOS 10.14.6 (18G95)

Thanks for looking into the clock. =)

@LaurenWags
Copy link
Member

LaurenWags commented Oct 8, 2019

Verified passed with

Brave 0.70.110 Chromium: 77.0.3865.90 (Official Build) beta (64-bit)
Revision 58c425ba843df2918d9d4b409331972646c393dd-refs/branch-heads/3865@{#830}
OS macOS Version 10.13.6 (Build 17G5019)

Screen Shot 2019-10-08 at 1 01 37 PM

Change to Portuguese-Brazil and relaunch:

Screen Shot 2019-10-08 at 13 02 39

Verification passed on

Brave 0.70.111 Chromium: 77.0.3865.90 (Official Build) beta (64-bit)
Revision 58c425ba843df2918d9d4b409331972646c393dd-refs/branch-heads/3865@{#830}
OS Ubuntu 18.04 LTS

Verification passed on

Brave 0.70.111 Chromium: 77.0.3865.90 (Official Build) beta (64-bit)
Revision 58c425ba843df2918d9d4b409331972646c393dd-refs/branch-heads/3865@{#830}
OS Windows 10 OS Version 1803 (Build 17134.1006)

image

image

@c-kirkeby
Copy link

This is still an issue on Windows 10 Pro 1909 (18363.476) in Version 1.0.0 Chromium: 78.0.3904.97 (Official Build) (64-bit). At 15:18 the new tab page is still showing 3:18.

@survivor303
Copy link

Perhaps people dosnt want to change from US to UK. Like me, i want that every new bits are there on my builds, so im not want wait for uk translations. Thats why i and many others want to use US language on primary everywhere but still want to change date formats and such to match our own ones.

This isnt just a brave problem but wide ratio to other applications too. why? it is a problem by design and then by devs who design software, they dosnt even think about this :/

@luminoso
Copy link

same here, on linux. won't it be a lot easier to make it a option on the new tab definitions?

@bsclifton
Copy link
Member

For folks experiencing this problem, check out #7951 and give it a +1 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment