Allow user to change time formats (12/24-hour clock) #9093
Conversation
sgiehl
added
Pull Request WIP
sgiehl
removed
the
Pull Request WIP
| protected function getTimeFormat() | ||
| { | ||
| if (is_null(self::$use12HourClock)) { | ||
| self::$use12HourClock = LanguagesManager::uses12HourClockForCurrentUser(); |
There was a problem hiding this comment.
Ideally, core would not know about anything implemented by plugins and not access any code from plugins directly. I know it's sometimes not really possible though or it is possible but doesn't make it much better. @diosmosis should we maybe use DI in such a case? Like by default core would always return true for date.uses12HourClockForCurrentUser DI setting (global.php) and LanguagesManager plugin would overwrite it?
There was a problem hiding this comment.
We should avoid using DI to store setting info, since settings can change in the middle of a request while DI is meant to be static for the whole request. Ie, if we change this setting in the middle of a request, there's no way to re-inject the objects that have already been created.
Instead, what I'd do is:
- Create a DateTimeFormatProvider service class in core to return individual formats. This class would provide default translations w/o going through the translator.
- Create a new DateTimeFormatProvider in the Intl/LanguagesManager plugin (whichever provides the best behavior when one of the plugins is not loaded) which uses the Translator to provide the formats.
- Override the DateTimeFormatProvider key in the plugin's config.php to use the plugin's DateTimeFormatProvider.
- Use the DateTimeFormatProvider here.
I'd probably also move the date/time formatting to MetricsFormatter (& move MetricsFormatter to DI as well), however that's not really required at the moment. And I'll have to do something like that to get rid of StaticContainer eventually, anyway.
There was a problem hiding this comment.
Ok, I don't think it would be a problem here if it changes during the request but for some things it can be a problem for sure.
There was a problem hiding this comment.
Tried to implement something similar now (see db969cd), feels better that way :)
|
Ready for another review. I've changed the mentioned stuff... |
|
DI related code looks ok to me |
|
👍 |
Allow user to change time formats (12/24-hour clock)
|
👍 |
mattab
added
the
answered
Add possibility to change time formats in user settings.
Besides the language it's now possible to choose the time format:
That will make 12- and 24-hour time formats available for all languages.
24-hour clock format will always be the default for every user.
fixes #9082