The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
Enabling the 'Show "All Day" instead of midnight' field formatter setting breaks the display of start/end times for non all-day date values.
This definitely affects Smart Date fields but it seems other Date fields may be affected as well.
Steps to reproduce
- Add a smart date field to an event content type
- Configure the field to use AP Style field formatter with the "Show 'All Day' instead of midnight" field formatter setting enabled
- Create an event with a start and end time (do not check "All Day")
- View event and see no start/end times displayed.
Proposed resolution
Fix the condition block that hides the time when the "all day" field formatter setting is enabled.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 3272760-7.patch | 1.2 KB | joegraduate |
Issue fork date_ap_style-3272760
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
joegraduateComment #4
trackleft2New patch with minimal changes to fix the issue.
Comment #5
joegraduateAlthough I think the refactoring changes included in @trackleft2's MR are good improvements, they may be better to address in a separate/follow-up issue. It would also be good to add a test for this regression.
I think the simplified patch in #4 would probably be good to get committed ASAP, if possible.
Comment #6
joegraduateFWIW, it looks like this regression was introduced with the changes made for #3167284: Support Smart Date field type in RangeFieldFormatter.
Comment #7
joegraduateLooks like the patch in #4 is missing a new line at the end of the file. The attached updated version should apply cleanly.
Comment #9
joegraduateNo actual tests exist so setting back to needs review.
Comment #10
mark_fullmerI can confirm that the patch in #7 fixes the missing time on our Drupal distribution, which uses Date AP Style in conjunction with Smart Date. I used our site's demo content to verify that the time displays correct on multiple date variation, including recurring events.
I also follow and agree with the logic change in #7's patch.
I'd need to spend more time going through the more substantial business logic changes in the merge request. Aside from the syntax cleanup and abstractions into helper methods, is there a different date rendering scenario that the merge request is addressing that is not covered in #7?
Comment #11
trackleft2@mark_fullmer the end result of the $time_output variable is exactly the same between the patch and the merge request.
I think we should probably use the pattern used in the patch in the merge request, since it is a bit simpler to understand.
The main difference between the merge request and the patch is that the merge request returns earlier once the output is determined to be an "All day" string.
Comment #12
mark_fullmerI am in support of the implementation proposal in #11. Confirming once again that this works with the latest version of smart_date `4.0.x` (as of 2023-07-20).
Setting to "Reviewed and tested by the community."
Comment #13
shelaneChris@trackleft2, I’m a little lost in what you’re saying in #11. Can you commit the implementation you think is best?
Comment #15
shelane