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

Date Time field type #1895

Closed
squigglybob opened this issue Dec 13, 2022 · 10 comments · Fixed by #1901
Closed

Date Time field type #1895

squigglybob opened this issue Dec 13, 2022 · 10 comments · Fixed by #1901
Assignees

Comments

@squigglybob
Copy link
Collaborator

It's time for a new field type :)

The problem

DT currently has a date field type, but it can only set the date, not the time.
Prayer Global laps care about the time as well as the date, but if the date is changed in the DT backend using the datepicker it will set the time to be midnight of that date, thus overwriting any time aspect of the timestamp that was originally there.

For custom laps that say want to start at 11am on a Sunday morning this is an issue if they are subsequently edited in DT.

Implementations in plugins

The trainings plugin implements a DateTime field type, but this is not currently in core.

The Prayer.Global porch needs a DateTime field for the lap start/end datetime.

Zume 5.0 is going to need a DateTime field as well.

Proposal

I propose adding in a new field type using the html input type <input type="datetime-local" />

This will be useful across several projects and hopefully also within the DT theme itself.

Time estimate

The number field took about 8 hours to implement back when I was still finding my way around.

I have left myself a trail of tagged comments of places in the code that need updating to add a new field type, so this should take significantly less time to implement. It's also hopefully not too much more complicated than the number field type

I estimate it to take 1 or 2 days at most.

@corsacca @ahillbilly what say you?

@corsacca
Copy link
Member

Can we just add a time param to the date field?
If time = true, then also include time.

@micahmills
Copy link
Collaborator

I saw this today an thought I would add my 2 cents. It has been a bit since I work on the date field, but I think it should be possible to extend that to include a time field. That being said, this is probably easiest done in web components and we can just add a time attribute to the dt-date component. It depends on your time line for needing this in core as to wether you implement it now or we implement it in the web components and then role those into DT.

@squigglybob
Copy link
Collaborator Author

Can we just add a time param to the date field? If time = true, then also include time.

That could work. It's literally just a UI semantics thing, as underneath it's a unix timestamp anyway...

@squigglybob
Copy link
Collaborator Author

I saw this today an thought I would add my 2 cents. It has been a bit since I work on the date field, but I think it should be possible to extend that to include a time field. That being said, this is probably easiest done in web components and we can just add a time attribute to the dt-date component. It depends on your time line for needing this in core as to wether you implement it now or we implement it in the web components and then role those into DT.

What would be the timescale on this?

Prayer.Global could do with it pretty soonish.
Zume probably a bit later than that.
(sorry for being so time specific ;) )

@squigglybob
Copy link
Collaborator Author

I always get overoptimistic when I look at the default view of caniuse and see green along the bottom line 🙄

Looking at the date version, the datetime-local input type has only got recent support in safari and firefox so probably isn't suitable for the time being.

image

What thoughts?

A time toggle in the current field type that toggles on an <input type="time" /> which then saves a time to the record?

Or just a new field type for doing time?

@micahmills
Copy link
Collaborator

micahmills commented Dec 15, 2022

It depends on what the fallback for the datetime-locale is. If the fallback is useable it might work.

If not I would add it as an option on the current date type.

@squigglybob
Copy link
Collaborator Author

The fallback according to mdn, is an input text box which I can confirm by trying the inputs on this page on an older than latest Safari browser

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/datetime-local

@micahmills
Copy link
Collaborator

micahmills commented Dec 15, 2022 via email

@squigglybob
Copy link
Collaborator Author

I think the daylight savings thing, is more that the input should allow users to choose invalid dates, which is the same as if we do a seperate date and time thing.

The daylight saving thing is then something we would have to think about when saving that time.

I'm definitely leaning in the direction of a seperate date time thing though

@micahmills
Copy link
Collaborator

micahmills commented Dec 16, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants