Problem/Motivation

If you have a date range configured to only store dates, and you use the daterange_duration widget, you get sort of confusing UI behavior.

A) If you define a date with the start as '2020-08-18' and say you want the duration to be '1 day', the widget (which was written to assume datetime, not just date) thinks the duration is an offset, and saves the end time as 2020-08-19. This isn't what users (the non-nerds who don't count from 0) expect. ;) If I say "1 day" I'd expect the start and end on the same day. If I say "2 days", starting today, I'd expect it to end tomorrow. Etc.

B) The widget settings still let you pick a duration granularity less than 1 day, which makes no sense for fields of this type.

Steps to reproduce

  1. Install core's datetime_range module.
  2. Install datetime_extras.
  3. Configure a node type with a daterange field, and select 'Date only' for the field storage setting.
  4. Configure the node type's form display mode to use the 'Daterange duration' widget.
  5. Create a node of this type and experience the weirdness.

Proposed resolution

For date-only range fields:

A) Always remove 1 day from the specified duration when computing the end date. When computing the default value for the widget, always add a day. That way, we always treat the 'duration' as the quantity of days in the range, not an 'offset' from the 1st day.

B) Enforce that the duration granularity is 1-day minimum, hide hours, minutes + seconds from the widget settings.

Remaining tasks

  1. Do it.
  2. Add test coverage for date-only ranges.
  3. Upload patch.
  4. Reviews / refinements.
  5. RTBC.
  6. Commit.

User interface changes

UI makes more sense for date-only ranges, where the 'duration' is a quantity of days in the range, not an offset from the starting date.

API changes

TBD, hopefully none.

Data model changes

None. We're not storing anything differently, just tweaking the UI of the widget.

CommentFileSizeAuthor
#7 3165990.6_7.interdiff.txt680 bytesdww
#7 3165990-7.patch16.69 KBdww
#7 3165990-7.test-only.patch5.53 KBdww
#6 3165990.4_6.interdiff.txt660 bytesdww
#6 3165990-6.patch17.1 KBdww
#6 3165990-6.drupalci-only.patch18.19 KBdww
#4 3165990.3_4.interdiff.txt8.38 KBdww
#4 3165990-4.patch17.09 KBdww
#4 3165990-4.test-only.patch5.95 KBdww
#3 3165990.2_3.interdiff.txt539 bytesdww
#3 3165990-3.patch9.49 KBdww
#3 3165990-3.test-only.patch5.53 KBdww
#2 3165990-2.patch9.49 KBdww
#2 3165990-2.test-only.patch5.53 KBdww

Issue fork datetime_extras-3165990

Command icon 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:

About issue forks

Comments

dww created an issue. See original summary.

dww’s picture

dww
we/he/they
commented
Status: Active » Needs review
StatusFileSize
new 3165990-2.test-only.patch5.53 KB
new 3165990-2.patch9.49 KB

This solves (and tests) point A.
Working on point B next.

dww’s picture

dww
we/he/they
commented
StatusFileSize
new 3165990-3.test-only.patch5.53 KB
new 3165990-3.patch9.49 KB
new 3165990.2_3.interdiff.txt539 bytes

Not sure what the bot's problem is there. :/ If you click through to Jenkins, it looks like things actually worked.

Fixing the CS bug, and trying again. /shrug

dww’s picture

dww
we/he/they
commented
StatusFileSize
new 3165990-4.test-only.patch5.95 KB
new 3165990-4.patch17.09 KB
new 3165990.3_4.interdiff.txt8.38 KB

Thanks to @mixologic in Slack, it seems the "Build successful" result is from our drupalci.yml saying we're supposed to halt-on-fail for phpcs failures, and we've got that one line that doesn't pass. Trying again with that line fixed.

Meanwhile, this fixes point B about the settings form for date-only fields. It's quite yucky, since we really need different default settings values depending on datetime vs. date-only, but defaultSettings() is public static. Alas. No tests for this part (yet).

The last submitted patch, 4: 3165990-4.test-only.patch, failed testing. View results

dww’s picture

dww
we/he/they
commented
StatusFileSize
new 3165990-6.drupalci-only.patch18.19 KB
new 3165990-6.patch17.1 KB
new 3165990.4_6.interdiff.txt660 bytes

Wee, progress!

Let's try exactly #4 again with a fix to drupalci.yml to stop failing runs on phpcs troubles.

But let's also fix the CS problem. ;)

dww’s picture

dww
we/he/they
commented
StatusFileSize
new 3165990-7.test-only.patch5.53 KB
new 3165990-7.patch16.69 KB
new 3165990.6_7.interdiff.txt680 bytes

Oh right, except we can't fix that phpcs "bug" since it *needs* to be /** @noRector */ for that to work. See #3137619: Fix @noRector comments to actually work

So here's #6 without that change...

The last submitted patch, 7: 3165990-7.test-only.patch, failed testing. View results

sickness29 made their first commit to this issue’s fork.

sickness29 opened merge request !3

sickness29’s picture

sickness29 commented
Status: Needs review » Fixed

Thanks @dww, merged

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

dww’s picture

dww
we/he/they
commented

For anyone else coming here wondering what happened, this was committed to 8.x-1.x, but the commit message didn't follow any of our conventions so there's no issue NID in the history, and therefore, no link from this issue. But it's this one:

commit bd726657071c0afd75cb51fd718406ea7907e199
Author: Andrew Sereda <38277-sic@users.noreply.drupalcode.org>
Date:   Mon May 6 13:41:39 2024 +0000

    Date only widget test; offset fix; increment setting changes for date only

https://git.drupalcode.org/project/datetime_extras/-/commit/bd726657071c...

That commit is included in the 8.x-1.2 release. Phew. 😉