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

Events Page: Added option to override browser time format and style #5538

Merged
merged 12 commits into from
Feb 22, 2023

Conversation

sinamics
Copy link
Contributor

Implements #5476

new optional ui config entries:

ui:
  use12hour: false     # true | false
  dateStyle: 'short'   # 'full' | 'long' | 'medium' | 'short
  timeStyle: 'medium'  # 'full' | 'long' | 'medium' | 'short

Would be nice if someone else could test and provide feedback.
one issue, when "full" is used (highly unlikely) it will clutter the mobile view, but at least it gives the user the option.

A few examples:

ui:
  dateStyle: 'full' 
  timeStyle: 'full'

image

(default)
ui:
  dateStyle: 'short' 
  timeStyle: 'medium'

image

ui:
  use12hour: true
  dateStyle: 'short' 
  timeStyle: 'medium'

image

ui:
  use12hour: true
  dateStyle: 'short' 
  timeStyle: 'short'

image

@netlify
Copy link

netlify bot commented Feb 18, 2023

Deploy Preview for frigate-docs canceled.

Name Link
🔨 Latest commit f9f8ba7
🔍 Latest deploy log https://app.netlify.com/sites/frigate-docs/deploys/63f2304d97276100082b9fd2

@NickM-27
Copy link
Collaborator

Are the date separators still inherited from the language?

@NickM-27 NickM-27 linked an issue Feb 18, 2023 that may be closed by this pull request
@sinamics
Copy link
Contributor Author

sinamics commented Feb 18, 2023

Are the date separators still inherited from the language?

damn, your are fast. 👍
I tested using 'en-US' instead of 'nb-NO' as language and it changes the date format to M/d/yyyy.
tested on Edge, Chrome, Firefox

@ehn
Copy link
Contributor

ehn commented Feb 18, 2023

I think this only captures a few of the date/time formats people want. For example, if you're outside of the US you probably don't want M/d/yyyy even if you want your UI in English. This won't let you set your date/time formats independent of the language.

How about letting the user specify an explicit strftime() format string? Alternatively, specifying another POSIX locale for LC_TIME?

@sinamics
Copy link
Contributor Author

Take 2
Modified based on @ehn comment. I do see the usecase to explicit define the date format in some cases and strftime would satisfy all users due to the flexibillity.

I was indecisive about whether to keep the previous configuration or switch entierly to strftime. After evaluating the added complexity that comes with using strftime, I decided to keep both solutions for the time being. Some users may only want to utilize use12hour.

config:

ui:
  use12hour: false # true | false
  dateStyle: 'short' # 'full' | 'long' | 'medium' | 'short
  timeStyle: 'medium' # 'full' | 'long' | 'medium' | 'short
  # NOTE! If strftime_fmt are provided it will takes precedence over the others
  strftime_fmt: '%A, %B %e, %Y %I:%M:%S %p %Z'

couple of thoughts

  • In order to provide more local-specific formatting, strftime supports a few locales. The locale for the user's browser will be used if e.g full time pattern format is specified with the day in clear text.
  • To support the use of strftime in the frontend, the strftime (MIT) npm package has been included in the project.
  • If strftime_fmt are provided it will takes precedence over the others
  • strftime_fmt config entry has no validation for now, so if users type in "hello world" it will be shown in the datetime field. I find it tricky to generate a reqex to validate the string as there is so many diffrent variation of the strftime format.

frontend

    // use strftime_fmt if defined in config file
    if (strftime_fmt) {
      const strftime_locale = strftime.localizeByIdentifier(locale);
      return strftime_locale(strftime_fmt, date);
    }

    // else use Intl.DateTimeFormat
    const formatter = new Intl.DateTimeFormat(locale, {
      dateStyle,
      timeStyle,
      timeZone: timezone || Intl.DateTimeFormat().resolvedOptions().timeZone,
      hour12: use12hour !== null ? use12hour : undefined,
    });
    return formatter.format(date);

frigate/config.py Outdated Show resolved Hide resolved
frigate/config.py Outdated Show resolved Hide resolved
frigate/config.py Outdated Show resolved Hide resolved
@blakeblackshear
Copy link
Owner

We don't use camelCase for any other properties in the config. Lets switch these to underscores for consistency.

Copy link
Collaborator

@NickM-27 NickM-27 left a comment

Choose a reason for hiding this comment

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

This option needs to be added to the docs. I would also suggest linking to https://www.gnu.org/software/libc/manual/html_node/Formatting-Calendar-Time.html so users are aware how it works

@NickM-27
Copy link
Collaborator

Have to say though, I wasn't super interested in this until I started playing with it but it is really nice being able to fully customize how the dates / times are presented

@sinamics
Copy link
Contributor Author

i will be away for the next week, but will update the docs when im back.

@NickM-27 NickM-27 self-requested a review February 22, 2023 13:54
@NickM-27 NickM-27 merged commit 3611e87 into blakeblackshear:dev Feb 22, 2023
@ehn
Copy link
Contributor

ehn commented Mar 14, 2023

There seems to be a bug here. If you set strftime_fmt (in my case to "%Y-%m-%d %H:%M:%S"), the presentation ignores the timezone setting (in my case Europe/Stockholm).

Please let me know if it's better to open a new issue than discuss it here.

@brunopiras
Copy link

There seems to be a bug here. If you set strftime_fmt (in my case to "%Y-%m-%d %H:%M:%S"), the presentation ignores the timezone setting (in my case Europe/Stockholm).

Please let me know if it's better to open a new issue than discuss it here.

Me too, without strftime_fmt the time is set to timezone!

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.

Set time format in Frigate rather than having to setup browser.
5 participants