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

Time everywhere in UI should be 24h #80

Closed
hmpf opened this issue Jun 23, 2020 · 3 comments
Closed

Time everywhere in UI should be 24h #80

hmpf opened this issue Jun 23, 2020 · 3 comments
Labels
discussion Requires developer feedback/discussion before implementation

Comments

@hmpf
Copy link
Contributor

hmpf commented Jun 23, 2020

Is this 24h time?

Screenshot_2020-06-23 React App

How are you supposed to create a duration that is 24h.

Screenshot_2020-06-23 React App(1)

@hmpf hmpf added the discussion Requires developer feedback/discussion before implementation label Jun 23, 2020
@jorgenbele
Copy link
Contributor

jorgenbele commented Jun 23, 2020

The start of day and end of day is defined in the backend here: https://github.com/Uninett/Argus/blob/1bcee6aaeb2e04f2e26b394d349f49bc2a510422/src/aas/notificationprofile/models.py#L61

https://github.com/Uninett/Argus/blob/1bcee6aaeb2e04f2e26b394d349f49bc2a510422/src/aas/notificationprofile/models.py#L60-L62

class TimeInterval(models.Model):
    DAY_START = time.min
    DAY_END = time.max

where time is imported from datetime here:
https://github.com/Uninett/Argus/blob/1bcee6aaeb2e04f2e26b394d349f49bc2a510422/src/aas/notificationprofile/models.py#L2

from datetime import datetime, time

Evaluating time.min and time.max in the python interpreter gives the following:

datetime.time(0, 0)
datetime.time(23, 59, 59, 999999)

I'm not really sure how this could be solved in the frontend. It could be solved by handling a special case of 00:00 (AM/PM), or by making the range: start <= x < round_up(end) such that 11:59 includes 11:59:59:9999.

In addition, what about timezones? I cannot see a way to set timezone, so how does it determine one?
EDIT: The backend handles timezones at the same time that it checks if it's within the time interval: https://github.com/Uninett/Argus/blob/1bcee6aaeb2e04f2e26b394d349f49bc2a510422/src/aas/notificationprofile/models.py#L95-L100, by getting the server timezone.

@hmpf
Copy link
Contributor Author

hmpf commented Jun 25, 2020

There might be a checkbox with "All day"?

@hmpf
Copy link
Contributor Author

hmpf commented Jun 25, 2020

How dates and times and timezones are shown differs between browsers and browser versions. Some use CLDR. Some use OS locales. Some you can set time and date formats in the setinngs, independently. It's a bit like the old difficulty with getting firefox to use A4, messy and unpredictable, which is why it is nice UX to allow people to control it themselves and not rely on the browser/os/moon phase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Requires developer feedback/discussion before implementation
Projects
None yet
Development

No branches or pull requests

2 participants