Hide datetime time field element by default

[mstenta]

This is something I've proposed a number of times in the past, and now that #635 has been merged it became easy to sketch up a proof of concept for consideration.

The idea is: on datetime fields, hide the "time" field by default, and replace it with a small "Show time" link.

Before:

After:

(clicking "Show time" causes it to look like the "Before".)

This would affect all datetime form elements, including the timestamp field in log edit forms, the planting quick form, and the bulk action forms.

This is a simple change, but I think it serves to improve UX in a few subtle ways. It makes the UI cleaner/simpler for broad cases where time isn't necessary most of the time (eg: cloning/rescheduling logs, planning animal movements and plant seeding/transplanting/harvest, etc), but it still allows specifying the time in a more granular way when necessary.

Here are two places this idea was mentioned in the past:

• https://farmos.discourse.group/t/reduce-date-time-entry-to-date-only-entry/415/3
• Timestamp widget hard to use #223 (comment)

Quotes from users:

When creating a log entry, very often the time when this event happened doesn’t matter to me. E.g. I think it’s not important at which time of the day I plowed a field. However, I can image that there are events for which the time is relevant. (still I think we could argue about the necessity of “seconds”).
Would there be a way that the time fields can be skipped with e.g. a checkbox that says “do not track time” or something?
I know, I can just leave the time that is entered by default or specify 0:00:00 as time but still I saw this fields confusing less tech-savvy users.

https://farmos.discourse.group/t/reduce-date-time-entry-to-date-only-entry/415

I agree, as a brand newbie 24 hours in to FarmOs I am finding the time setting unnecessarily detailed. Perhaps I am missing something obvious though. Most times I will be setting records to a date. Sometimes wanting to include the hour and rarely if ever the minute and seconds.

https://farmos.discourse.group/t/reduce-date-time-entry-to-date-only-entry/415/2

[symbioquine]

Reposting here what I posted in the Matrix chat...

I'm trying to understand the consequences of #635 and #637 together in terms of the ordering of things like movements...

It seems like although the ordering would still work (by falling back on the order of the log ids), it would end up being less intuitive because it users wouldn't see the obvious "this log occurs after this other log" relationship as clearly.

I think it is still valuable to set the time in many scenarios even when the user mostly "doesn't care" about it.

Another problematic scenario is when the user selects a date in the past for a new log - because it is probably unintuitive for that log to (by default) "come before" other logs on that date.

(All this is assuming I'm understanding the effects of those changes correctly.)

[mstenta]

I responded in chat (let me know if I missed anything or there are still questions @symbioquine!).

Copying some of the point here for posterity:

@symbioquine: I think it is still valuable to set the time in many scenarios even when the user mostly "doesn't care" about it.
@mstenta: Existing behavior doesn't change (if i'm understanding your thoughts), the time field just gets hidden with a link to show it. The default value of the time is the same with all these changes.
@symbioquine: Hmmm, I guess I'm probably misunderstanding #635 then...
@symbioquine: Okay, maybe I'm seeing what's going on better now... #635 probably does have that effect, but only in some specific cases like the planting quick form and the bulk "move"/"group" assets actions. Those places are only a subset of the forms that #637 would affect - most of the later retaining their existing behaviour of using the current time.
@symbioquine: Does that sound right?
@mstenta: Yea that's correct... and date('Y-m-d', ...) is the same as "midnight" so those didn't change
@mstenta: Basically it was taking the current date (without time) and converting it to a timestamp
@symbioquine: I guess that makes sense
@mstenta: The change kinda just made it more explicit by actually saying "midnight" :-)
@mstenta: And yea all the other forms still work the same
@symbioquine: Cool

[mstenta]

I copied two relevant quotes from users into the PR description above for reference.

[mstenta]

It's probably important to emphasize that this doesn't change current behavior of the date/time fields at all. It still loads the forms with a time of either 00:00:00 (eg: bulk actions, quick forms, etc) or the current time (eg: when you create a new log).

I chose the string "Show time" because I think that is the simplest, while still conveying that the time is there... it's just hidden.

One thing that still needs to be done (meant to but forgot) is wrap the "Show time" string in the Drupal JS translation function, so it gets translated properly.

[mstenta]

This PR would exacerbate the issue described in #625 - so we might want to think about a solution to that at the same time.

Basically: there is an existing UX issue where "optional" (non-#required) datetime fields cause a validation warning if you only fill in a date, but not a time.

[mstenta]

Quick notes from call w/ @paul121 (some related to this, some semi-related):

• automatically set time to 0s if date is filled on submit (fixes Date format error on test logs. #625)
• provide an option to show/hide time field on a per-field basis
• if time IS set, always show time

theory: if time is 0s, i don't care about it

[paul121]

provide an option to show/hide time field on a per-field basis

I've added a #toggle_time option that must be set to TRUE before on the datetime form element before this behavior is applied.

Refactor datetime toggle to use data-farm-toggle-time attribute34fb77e

This fixes a couple bugs I found. One is that hard-coding style.display = 'block' when showing the time may not always be correct... in fact with Gin, it seems to use an inline-flex display. Using the css display: none property solves this and should be more robust.

Also, I believe as currently implemented this JS behavior would effect all datetime form elements on the page, not only specific ones... by targeting the data-farm-toggle-time attribute we ensure only the desired form elements are modified.

if time IS set, always show time

I added some logic for this, but unfortunately it only works for the datetime_timestamp_optional field widget itself. I think we could move this logic into function farm_ui_theme_preprocess_datetime_form(&$variables), it will just be a bit challenging to get the field's value. It's much more convenient to do this in the field widget. Perhaps that is a good start for now?

This also brings up an issue with how I've implemented things: farm_field would have a dependency on farm_ui_theme. I think this is kinda inevitable for adding this feature to field widgets... unless we move the datetime_toggle library down outside of the UI module. IMO it would be OK living somewhere like farm_field because it doesn't have any dependencies on Gin, if anything it has dependencies on deeply rooted admin theme templates for fields + datetime library. I would expect that someone building their own admin theme would use these same patterns and this JS would likely continue to work.

automatically set time to 0s if date is filled on submit (fixes #625)

Still not quite sure how we can fix this... I didn't spend much time looking at this but not sure how we can modify the submit logic for the datetime form element without creating our own. Maybe a custom #value_callback or #process callback?

[mstenta]

Thanks @paul121! These are great next steps. Still more to do, but it's getting closer!

I tested locally and these changes serve to target the following cases:

• Planting quick form
• Animal move/group action forms

The cases that it does not cover are:

• Log form timestamp fields
• Log clone/reschedule action forms
• FarmFieldFactory timestamp fields that are not required (eg: Animal birthdate) - because the time is empty by default, so $existing_time_midnight is FALSE (although this relates to "automatically set time to 0s if date is filled on submit (fixes Date format error on test logs. #625)")

This also brings up an issue with how I've implemented things: farm_field would have a dependency on farm_ui_theme.

I see what you mean. We certainly can't have low-level modules depending on UI modules. Moving it down to farm_field might make sense - at least as a first step. It feels weird to me because that introduces UI logic in a low-level module - but farm_field is a bit weird in that regard because it does make UI decisions about field widgets etc. And most higher-level modules already depend on it (directly/indirectly including the three affected by your changes: farm_location, farm_group, farm_quick_planting).

If we moved it to farm_field it would mean that it becomes impossible to disable this behavior in core elements, though. Maybe that's OK?

I don't love the fact that it would add a bunch of new files to farm_field just for this UI enhancement, though (farm_field.libraries.yml, farm_field.module, datetime_toggle.css, datetime_toggle.js).

What about this (thinking from the other direction): could we make the "toggle time" behavior default to TRUE? And apply it to all datetime elements that DON'T explicitly set #toggle_time to FALSE? I'd have to take a closer look to see how that would work, but maybe it would just require a bit of refactoring of the JS? And we could remove all the `'#toggle_time' => TRUE's?

automatically set time to 0s if date is filled on submit (fixes #625)
Still not quite sure how we can fix this... I didn't spend much time looking at this but not sure how we can modify the submit logic for the datetime form element without creating our own. Maybe a custom #value_callback or #process callback?

I think we should figure this out before merging, either in this PR or another one, because it is indirectly related and these changes will exacerbate #625.

Hmm...

[mstenta]

Minor tweak to make this language a bit clearer?

Default to time input hidden with a button to show time input. Hide the time input by default, with an option to show it.

[mstenta]

Minor: Rename to Drupal.behaviors.farm_ui_datetime_toggle?

[mstenta]

could we make the "toggle time" behavior default to TRUE? And apply it to all datetime elements that DON'T explicitly set #toggle_time to FALSE?

I think this would cover the "Log form timestamp fields" and "Log clone/reschedule action forms"... ?

[paul121]

could we make the "toggle time" behavior default to TRUE? And apply it to all datetime elements that DON'T explicitly set #toggle_time to FALSE?

I think we should avoid this. It would modify datetime form elements everywhere, being used by core or any other module. This would be a convenient way to solve the log timestamp and log action forms but I think there are too many other unknowns.

As an example: the Rothamsted quick forms use datetime and expect the user to enter a time value. I'd be happy to add a toggle_time = FALSE for that use case, but it's impossible to know what other use cases this might affect.

[mstenta]

Makes sense.

If we move this stuff to farm_field it means we'd also have to add form alters there to handle the Log form/action fields - which feels weirder and weirder to me. Unless we do that in a UI module, but then we have this stuff in multiple places.

Tricky tricky.

[mstenta]

I suppose we could alter the #toggle_time into all the "default" places we want within farm_ui_theme too...

[mstenta]

I suppose we could alter the #toggle_time into all the "default" places we want within farm_ui_theme too...

Or at least the "low-level" ones, like farm_field and farm_location (and maybe farm_group because it's similar), and the log module overrides. We could leave #toggle_time in farm_quick_planting because it's "higher level".

[paul121]

I suppose we could alter the #toggle_time into all the "default" places we want within farm_ui_theme too...
Or at least the "low-level" ones, like farm_field and farm_location (and maybe farm_group because it's similar),

We could do all the alters... But this seems like a good reason to keep it as a lower level dependency and avoid that complexity! I see farm_field is already a dependency of farm_location and thus a dependency of farm_group as well.

Would still need to alter anything provided by the log module. But if we move all of this core functionality into out of farm_ui_theme to farm_field (making this less of a "UI" type feature) then I think we could do log alters in farm_log, which seems like a good place: Provides additional Log entity features for farmOS.

One step further, I'm curious about providing a custom datetime form element altogether like farm_datetime. A few reasons:

• This could provide two defaults for convenience: '#default_value' => new DrupalDateTime('midnight') and '#toggle_time' => TRUE.
• It would be more explicit that modules are adhering to the "farm datetime" features.
• Last, a farm_datetime element might make it easier to encapsulate the "0s" time validation/submission issue as well. I think the alternative would mean another #value_callback/validate/process attribute that developers need to add themselves. I'm not sure how we would "automatically alter" to add such an attribute for this "0s" time validation to existing datetime elements without creating similar issues. It certainly goes in hand with toggle_time but is also a distinct issue when toggle_time isn't enabled.

But we need to think of what downsides there might be to this. Things that come to mind are added maintenance & losing any other potential alters that target the datetime element (but... this is probably for the best?) FWIW my approach above alters the datetime_form template which I think we would still continue to use in a farm_datetime element