Problem/Motivation

The 7.x Date module allowed the field to has an optional or required end date. D8 requires the end date always.

Proposed resolution

Make end_value in DateRangeItem optional, add validation constraint via configuration.

Remaining tasks

User interface changes

API changes

Data model changes

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

webflo created an issue. See original summary.

mpdonadio’s picture

TBH, I am not terribly keen on this, because of the ripple effects is has. Any consideration with this will need to wait until some of the refactoring we need to do gets wrapped up, the views filters get finished, and the timezone work is finalized. I really want to avoid having to do an option check in every single method (in addition to the date / date+time one we already do, which is tech debt we have to deal with).

SKAUGHT’s picture

it certainly is expectable that an Event will have a start time and may not have a fixed end time.. this is a normal use case.

for both UX and data validation the good old 'has end date' checkbox may get good to bring back

mpdonadio’s picture

I guess the thing is that we now have two fields, datetime and daterange. If we make end dates optional in daterange, then we are essentially making the datetime field not needed because daterange will have all of it's features. I'm not saying we can't do this, just that this really does need to wait for other planned work to shake out first.

webchick’s picture

Status: Active » Postponed

Marking postponed, in that case. But I do agree that this seems like pretty essential functionality.

SKAUGHT’s picture

#4.
I certainly get we have both fields. I think the split to datetime_range during the last phase of dev caused this. indeed there is a lot of tech dept here. re-integration of these types would be more idea in the long run.

the sort term answer for site builders is to use a conditional field state for single or end-date ready. which always makes getting views, node renders and other related queries of dates horrible cause you have to check two fields. but at least now, there is this sort term approach.

dkosbob’s picture

As a workaround, I ended up defining a new FieldType class in a custom module that extends the DateRangeItem class, and setting Required to false there. I then used hook_field_info_alter to assign that new class to the daterange field. That allowed the end date to be optional. But since we don't actually want the end date to be left empty, I used hook_entity_presave to look for daterange fields and use the start date as the end date if the end date is left empty. In our theme, any daterange field with the same start date and end date is rendered as a single date.

The new class:

<?php

namespace Drupal\my_module\Plugin\Field\FieldType;

use Drupal\Core\Field\FieldStorageDefinitionInterface;
use Drupal\Core\TypedData\DataDefinition;
use Drupal\datetime_range\Plugin\Field\FieldType\DateRangeItem;

/**
 * Plugin implementation of the 'daterange' field type.
 *
 * @FieldType(
 *   id = "daterange",
 *   label = @Translation("Date range"),
 *   description = @Translation("Create and store date ranges."),
 *   default_widget = "daterange_default",
 *   default_formatter = "daterange_default",
 *   list_class = "\Drupal\datetime_range\Plugin\Field\FieldType\DateRangeFieldItemList"
 * )
 */
class MyModuleDateRangeItem extends DateRangeItem {

  /**
   * {@inheritdoc}
   *
   * The entire reason this class exists is to overwrite the Required property
   * of end_value. It is only safe to do this because in this module we fill the
   * end_value with the start_value when left empty. Keep an eye on:
   * https://www.drupal.org/node/2794481.
   *
   * @see my_module_agency_entity_presave();
   */
  public static function propertyDefinitions(FieldStorageDefinitionInterface $field_definition) {
    $properties = parent::propertyDefinitions($field_definition);

    $properties['end_value'] = DataDefinition::create('datetime_iso8601')
      ->setLabel(t('End date value'))
      ->setRequired(FALSE);

    return $properties;
  }

}

The field info alter and presave hooks, in my_module.module:

/**
 * Implements hook_field_info_alter().
 *
 * Take over the DateRangeItem class from datetime_range so that we can remove
 * the requirement on end_value.
 */
function my_module_field_info_alter(&$info) {
  if (isset($info['daterange']) && class_exists('Drupal\my_module\Plugin\Field\FieldType\MyModuleDateRangeItem')) {
    $info['daterange']['class'] = 'Drupal\my_module\Plugin\Field\FieldType\MyModuleDateRangeItem';
  }
}

/**
 * Implements hook_entity_presave().
 *
 * Populate end date on daterange fields with start date if empty.
 */
function my_module_entity_presave(EntityInterface $entity) {
  // Limiting this to nodes for the time being. But If a daterange field is used
  // on a different entity type and no end date is supplied, I think the site
  // might blow up, so figure that out.
  if ($entity->getEntityTypeId() !== 'node') {
    return;
  }

  $daterange_fieldnames = array();

  // Fill $daterange_fieldnames array with fieldnames of each daterange field.
  $fields = $entity->getFieldDefinitions();
  foreach ($fields as $fieldname => $field) {
    if (substr($fieldname, 0, 6) !== 'field_') {
      continue;
    }
    $fieldtype = $field->getType();
    if ($fieldtype !== 'daterange') {
      continue;
    }
    $daterange_fieldnames[] = $fieldname;
  }

  // Handle each daterange field.
  foreach ($daterange_fieldnames as $daterange_fieldname) {
    $daterange_field = $entity->get($daterange_fieldname);
    if ($daterange_field->isEmpty()) {
      continue;
    }

    $start_value = $daterange_field->get(0)->get('value')->getValue('value');
    $end_value = $daterange_field->get(0)->get('end_value')->getValue();

    // If end_value is left empty, set it same as start_value.
    if ($end_value === NULL || $end_value === '') {
      $daterange_field->get(0)->get('end_value')->setValue($start_value);
    }
  }
}

This isn't perfect and I'm sure it could be improved (please, do tell). But it seems to be working for us for the time being, until a more permanent solution is put in place.

cknoebel’s picture

Echoing Webchick in #5 with a use case I was working on when I hit this issue: I have a content type for alerts which has two levels for alerts, low and high. I know about low level alerts ahead of time and can schedule them to publish with a date range ("Elm Dr. closed for pothole repairs from 10:00 a.m. to 2:00 p.m.). In this case the date range lets me set and forget. The high level alerts come unplanned and continue for some time (I get a call that a building is on fire). In this case I have no end date because the alert is ongoing.

It would be great if the end date is optional for cases like this.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

daften’s picture

Thanks @dkosbob for the workaround. Just as a note, this only works for optional daterange fields, since it doesn't adapt the field widgets and the start and end date required flag are based on the field required property. Combined with browser based validation for required elements, you'd still have to fill it out.

gagarine’s picture

No end date isn't that the end date is the same that the start date? Don't know if they will be side effect on this approach. It's what dkosbob is doing.

But yes perhaps we really want to have the information that their is no information.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

hudri’s picture

+1 on this FR, in my current project I've exactly the use case as described in #3. As a workaround I had to move to two separate date fields, but I'm missing the endtime validation and the slick UI

colan’s picture

Status: Postponed » Active

Now that Date Range is done, can we bring this back to life by considering #4?

Possible steps:

  1. Make the end date optional in Date Range.
  2. Deprecate Date in D8.
  3. Rename Date Range to Date in D9.
mpdonadio’s picture

I'd entertain work on this now, but I consider it soft blocked on the views issues and maybe the time zone one. I also think we need to refactor the functional tests to be more manageable to avoid reroll madness. So, let's play with play with the non-test changes and get feedback on that before test coverage.

Right now, I think I am ok with two fields with some overlapping functionality.

kreatIL’s picture

+1

As webchick already stated: this is essential functionality. I'll try to give an example: a calendar of mixed events, some of them lasting several hours, others several days. Of again others is not known, except the door opening time, how long they will really last. So, if the end-value is a mandatory field, this can lead to incorrect information on the site, because the editor is forced to name an end time that he does not even know.

mpdonadio’s picture

yareckon’s picture

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Tichris59’s picture

A little workaround that works for me :

Create a php file in your module :

MODULE_NAME/src/Plugin/Field/FieldWidget/DateRangeCustomWidget.php

<?php

namespace Drupal\datetime_custom\Plugin\Field\FieldWidget;

use Drupal\Component\Utility\NestedArray;
use Drupal\Component\Utility\SortArray;
use Drupal\Core\Entity\EntityStorageInterface;
use Drupal\Core\Field\FieldDefinitionInterface;
use Drupal\Core\Field\FieldItemListInterface;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
use Drupal\datetime_range\Plugin\Field\FieldType\DateRangeItem;
use Drupal\datetime_range\Plugin\Field\FieldWidget\DateRangeWidgetBase;
use Symfony\Component\DependencyInjection\ContainerInterface;

/**
 * Plugin implementation of the 'daterange_custom' widget.
 *
 * @FieldWidget(
 *   id = "daterange_custom",
 *   label = @Translation("Date and time range custom"),
 *   field_types = {
 *     "daterange"
 *   }
 * )
 */
class DateRangeCustomWidget extends DateRangeWidgetBase implements ContainerFactoryPluginInterface {

  /**
   * The date format storage.
   *
   * @var \Drupal\Core\Entity\EntityStorageInterface
   */
  protected $dateStorage;

  /**
   * {@inheritdoc}
   */
  public function __construct($plugin_id, $plugin_definition, FieldDefinitionInterface $field_definition, array $settings, array $third_party_settings, EntityStorageInterface $date_storage) {
    parent::__construct($plugin_id, $plugin_definition, $field_definition, $settings, $third_party_settings);

    $this->dateStorage = $date_storage;
  }

  /**
   * {@inheritdoc}
   */
  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
    return new static(
      $plugin_id,
      $plugin_definition,
      $configuration['field_definition'],
      $configuration['settings'],
      $configuration['third_party_settings'],
      $container->get('entity_type.manager')->getStorage('date_format')
    );
  }

  /**
   * {@inheritdoc}
   */
  public function formElement(FieldItemListInterface $items, $delta, array $element, array &$form, FormStateInterface $form_state) {
    $element = parent::formElement($items, $delta, $element, $form, $form_state);

    // Identify the type of date and time elements to use.
    switch ($this->getFieldSetting('datetime_type')) {
      case DateRangeItem::DATETIME_TYPE_DATE:
      case DateRangeItem::DATETIME_TYPE_ALLDAY:
        $date_type = 'date';
        $time_type = 'none';
        $date_format = $this->dateStorage->load('html_date')->getPattern();
        $time_format = '';
        break;

      default:
        $date_type = 'date';
        $time_type = 'time';
        $date_format = $this->dateStorage->load('html_date')->getPattern();
        $time_format = $this->dateStorage->load('html_time')->getPattern();
        break;
    }

    $element['value'] += [
      '#date_date_format' => $date_format,
      '#date_date_element' => $date_type,
      '#date_date_callbacks' => [],
      '#date_time_format' => $time_format,
      '#date_time_element' => $time_type,
      '#date_time_callbacks' => [],
    ];


    $element['end_value'] += [
      '#date_date_format' => $date_format,
      '#date_date_element' => $date_type,
      '#date_date_callbacks' => [],
      '#date_time_format' => $time_format,
      '#date_time_element' => $time_type,
      '#date_time_callbacks' => [],

    ];

    $element['end_value']['#required'] = false;

    return $element;
  }

  /**
   * {@inheritdoc}
   */
  public function extractFormValues(FieldItemListInterface $items, array $form, FormStateInterface $form_state) {
    $field_name = $this->fieldDefinition->getName();

    // Extract the values from $form_state->getValues().
    $path = array_merge($form['#parents'], [$field_name]);
    $key_exists = NULL;
    $values = NestedArray::getValue($form_state->getValues(), $path, $key_exists);


    if ($key_exists) {
      // Account for drag-and-drop reordering if needed.
      if (!$this->handlesMultipleValues()) {
        // Remove the 'value' of the 'add more' button.
        unset($values['add_more']);

        // The original delta, before drag-and-drop reordering, is needed to
        // route errors to the correct form element.
        foreach ($values as $delta => &$value) {
          if( $value['end_value'] == null )
            $value['end_value'] = $value['value'];
          $value['_original_delta'] = $delta;
        }

        usort($values, function ($a, $b) {
          return SortArray::sortByKeyInt($a, $b, '_weight');
        });
      }

      // Let the widget massage the submitted values.
      $values = $this->massageFormValues($values, $form, $form_state);

      // Assign the values and remove the empty ones.
      $items->setValue($values);
      $items->filterEmptyItems();

      // Put delta mapping in $form_state, so that flagErrors() can use it.
      $field_state = static::getWidgetState($form['#parents'], $field_name, $form_state);
      foreach ($items as $delta => $item) {
        $field_state['original_deltas'][$delta] = isset($item->_original_delta) ? $item->_original_delta : $delta;
        unset($item->_original_delta, $item->_weight);
      }
      static::setWidgetState($form['#parents'], $field_name, $form_state, $field_state);
    }
  }





}

In formElement function I set end_value to required false
In extractFormValues i set end_value the same value as start_value if null.

Birk’s picture

I've made a workaround module, we're using while this issue is being resolved: https://www.drupal.org/project/optional_end_date

So far it's only implemented in a project still under development, so it's not yet tested on a live site.
It supports null values for the end date, so the we can work with date ranges without a specific end date (the default output is the same as with start and end being identical).

dalra’s picture

I think the best solution is to make the user CHOOSE either a temporary "Present" end date (as a checkbox) OR a fixed end date.
With JavaScript, when the user clicks on the checkbox, the end date input fields are hidden and the word "Present" is shown instead.

New York - 2016.04 - Present
London - 2011.02 - 2016.04

So either way, we still have a date RANGE (not a simple date in the case the end is optional OR a date range as it is now).
With this "Present" checkbox, we can always calculate the duration of the event. We can always enter a final date instead of "Present" by unchecking the checkbox.

The word "Present" should of course be changeable and translatable.

jonathanshaw’s picture

The #22 suggestion doesn't address what I see as one of the most common scenarios here:
describing a future event whose start date is known and whose end date is not (yet) know. 'Present' is completely inadequate in this case.

The most critical problem is: how to set things up so that content editors don't have to enter false information when they don't know the information.

mpdonadio’s picture

#21 posting your work as a patch against 8.6, too, would help this feature more forward.

dalra’s picture

See my comment below.

dalra’s picture

I have posted a reply which it is held for approval, so here is a better version of it:

Having the OPTION to show in the Frontend "a Date Range + a checkbox" instead of just "a Date Range" should solve the problem.

And when #2942828: Make the Date and Date Range configurable to allow MIN and MAX dates, only past or only future dates, or both is implemented, the date range will be very flexible.

My suggestion from #22 covered only one part: events which started in the past and are still happening (Present).

To also cover the other part, events with an unknown end (past unknown or future unknown), we can improve that suggestion.
In the Backend, in the date range type settings we can add an option to show Date Range + "Unknown end date" OR "Present".
If the end date was selected as Unknown, there needs to be an OPTION to show ONLY the start date in the Frontend when displaying the event's start and end dates (to hide the word Unknown).

In the Frontend, the user sees either:

Choose Start Date - Choose End Date + a checkbox "Unknown end date"
or
Choose Start Date - Choose End Date + a checkbox "Present"

When the checkbox is checked the "Choose End Date" field is hidden and replaced by the word "Present"(or "Unknown") using JavaScript.
So the "Choose End Date" field depends on the checkbox.

From what I can see, the modules for D8 which have implemented conditional fields are select_or_other, conditional_fields and business_rules.
I have tested only the last 2 and in both the checkbox selection doesn't work properly yet.

Birk’s picture

The attached patch implements what I've done in the optional_end_date module (mentioned in #21).

The idea is to optionally (checkbox added to the storage settings of the daterange field) allow NULL values for the end_value.
Default behavior for the formatters, when the end date is not filled, is to act as if start and end is equal (only show the start date).

It's purely functional, I've not looked at or altered any tests, it's a take on a possible solution.

mpdonadio’s picture

Status: Active » Needs review

Go testbot, go!

dalra’s picture

@Birk
It's great to see there is progress on this issue.
Would you consider also adding an option to show the checkbox in the front-end like I suggested in #26?
It would certainly make things more simple to understand for the users who fill the data.
So, in the front-end, there will be a choice between entering an end date or clicking a checkbox (which will hide the end date input fields).

mpdonadio’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests
+++ b/core/modules/datetime_range/src/Plugin/Field/FieldType/DateRangeItem.php
@@ -83,6 +92,12 @@ public function storageSettingsForm(array &$form, FormStateInterface $form_state
+      '#title' => t('Optional end date'),
+      '#default_value' => $this->getSetting('optional_end_date'),

Not sure if I like this label, but that is a nit.

NW for needing test coverage. It is possible, though, that we want to refactor DateRangeFieldTest first, since that is a beast of a test class. Adding coverage for datetime, dateonly, and allday for the widgets + formatters is going to nearly double the length of that class.

daften’s picture

I've noticed that when I set the field as required, even with optional end date, the end date is required in the front-end.
I think what @dalra proposes is maybe a bridge too far, but when the end date is optional, it shouldn't be required in the front-end.

dalra’s picture

My original proposal could be simplified (we do not keep the present day anymore) and reduced to this:

In the back-end, in the daterange options, next to the "Optional end date" checkbox, add another checkbox called "Show a checkbox in the front-end to hide the end date and show a label"
Of course, when this is checked in the back-end, the front-end user will see:

Input start date + input end date + a checkbox to hide the end date (eg: with the label "Unknown end date" or "Ongoing")

The label should be configurable in the back-end.

So in the fronted, when the user checks the "Unknown end date" checkbox, the end date input field is hidden with JavaScript and the label "Unknown end date" is shown.
If they unchecked it, the end date is shown again.
This eliminates any confusion the front-end user might have.
These 2 checkboxes (one in the back-end to show another one in the front-end) solve the problem.

If this checkbox is not implemented, it's just a bad user experience:
1) the user doesn't know if the end date is required
2) with only start date + end date, for an ongoing event, (like "Art gallery in New York": 2016.04 - still open) the user doesn't know what to do, if they should enter today's date (the wrong choice) or not to enter anything (the user is uncertain if the submission will work).
In this case the chosen label is "still open" instead of "Unknown end date".

mpdonadio’s picture

Issue tags: +Usability
+++ b/core/modules/datetime_range/src/Plugin/Field/FieldType/DateRangeItem.php
@@ -46,7 +55,7 @@ public static function propertyDefinitions(FieldStorageDefinitionInterface $fiel
-      ->setRequired(TRUE);
+      ->setRequired(!$field_definition->getSetting('optional_end_date'));

I believe this means we will need an update path. Not adding that tag until we have a solid approach, though, so we know what we will need. But, I think that is what caused the required problem described in #31.

The patch in #27gave me a fatal on a clean install against 8.6.x when I tried adding the node:

The website encountered an unexpected error. Please try again later.Drupal\Core\Entity\EntityStorageException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'field_daterange_end_value' cannot be null: INSERT INTO {node__field_daterange} (entity_id, revision_id, bundle, delta, langcode, field_daterange_value, field_daterange_end_value) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6); Array
(
[:db_insert_placeholder_0] => 1
[:db_insert_placeholder_1] => 1
[:db_insert_placeholder_2] => page
[:db_insert_placeholder_3] => 0
[:db_insert_placeholder_4] => en
[:db_insert_placeholder_5] => 2018-02-01
[:db_insert_placeholder_6] =>
)
in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 829 of core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).

Drupal\Core\Database\Statement->execute(Array, Array) (Line: 625)
Drupal\Core\Database\Connection->query('INSERT INTO {node__field_daterange} (entity_id, revision_id, bundle, delta, langcode, field_daterange_value, field_daterange_end_value) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6)', Array, Array) (Line: 87)
Drupal\Core\Database\Driver\mysql\Connection->query('INSERT INTO {node__field_daterange} (entity_id, revision_id, bundle, delta, langcode, field_daterange_value, field_daterange_end_value) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6)', Array, Array) (Line: 32)
Drupal\Core\Database\Driver\mysql\Insert->execute() (Line: 1341)
...

Re the approach described in #32, not sure if I like the NULL-prints-label thing or not.

Any approach we take will also need a proper Usability Review and an Accessibility Review.
dalra’s picture

Re the approach described in #32, not sure if I like the NULL-prints-label thing or not.

Well, #32 can be improved.

We show another checkbox in the back-end to make the JavaScript label optional.
Some users might want to just hide the end date input field when the box is checked, without showing a label. This will show only the start date and hide the end date.

Those who need the end date label, can enable it.

We have 2 labels here:
1) next to the front-end checkbox. This could be short (1-2 words) because it's next to the checkbox.
2) one shown instead of the end date field. Here we can put a few more words to make sure the user understands

We need to make both labels configurable in the back-end.
Some users might put the same text in both, but some might want to put a synonym, to make sure the front end user gets it

mpdonadio’s picture

+++ b/core/modules/datetime_range/src/Plugin/Field/FieldType/DateRangeItem.php
@@ -46,7 +55,7 @@ public static function propertyDefinitions(FieldStorageDefinitionInterface $fiel
-      ->setRequired(TRUE);
+      ->setRequired(!$field_definition->getSetting('optional_end_date'));

Reading through some of the earlier patches of #2161337: Add a Date Range field type with support for end date may shed some light on this problem, especially if some are seeing this work and other aren't (as it may appear). In that, we had problems using a dynamic schema in `DateRangeItem::propertyDefinitions()` based on the `$field_definition` settings. IIRC, this was because of when this method was invoked at different places (I would have to set some breakpoints again to be sure). I think we will have to `->setRequired(FALSE)` in the field definition, and then set required on the element in the widgets, based on the settings. Still need a update hook at the end, though.

Re #34, I am probably being dense, but I don't quite understand what is being described. A few things to keep in mind with this are

- JS can enhance the UX, but the UX and A11Y need to work properly w/o it. This is a firm core gate.
- With the datetime related widgets/formatters, we try to avoid the do-it-all scenario, and focus instead on building blocks that provide functionality, and then can be enhanced in contrib. We also don't need to do everything all at once. Optional end time has been identified as good additional functionality. I am leaning towards #32/#34 being deferred to a followup or contrib.

dalra’s picture

I have created some mockups from the patch in #27 and from my suggestions in #32 and #34.

End date optional

When checkbox 3 is checked:

End date optional

daften’s picture

@dalra: I think most of what you suggest is overkill for a lot of use cases, and should be handled in follow-up issues. The basic premise here should be: let's make it possible to have the end date optional.
What you suggest is a huge impact on what needs to be stored, would need a lot of reviews, e.g. for accessibility. While optional end date should be do-able quicker and easier.

yoroy’s picture

Issue tags: +Needs usability review
webchick’s picture

We talked about the proposal at #36 on the weekly usability meeting. There was consensus that the options provided in these mockups sound like a potentially neat contrib module like "date_time_advanced_config", but that for core, which aims for hitting the "80%" case, these sorts of options would feel like overkill.

Rationale:

- For all other Drupal fields that offer a "required" vs. "optional" (e.g. text, number, and so on), the data entry experience is to just leave the optional field blank and then it won't get printed upon display. Other fields do have the ability to define a default value, but not a "what to print here if someone leaves it blank" value, and that seems odd to add to this one field and not others.

- This also seems to match what in a quick check Google Calendar, Meetup, Facebook Events, iCal, etc. do. Those services don't actually let you pick an optional end date... they default to the current one, and if the from + to date match, they simply print the from date (this seems like very sensible behaviour for Drupal as well). There are no options to choose customized text to show instead; that would complicate the data entry quite significantly.

So, given the other Drupal fields and these other popular event management services will demonstrate generally expected behaviour, and the closer we can match them, the better off we'll be, I'm inclined to agree with @daften on this proposal being out of scope. But once again, feel free to create a contributed module that offers these options, if you feel people would benefit.

dalra’s picture

@daften
If the end date is optional, the only thing that is stored instead of it is NULL.
So there's no impact here.

In the mockups, I only suggested a few very simple things:

1) Show a simple checkbox in the front-end to improve usability and user experience.
2) Give a name (a label) to that checkbox. Don't show a box without a name.
3) Have an option to show a label when the end date is hidden. It's very useful also to the people who use screen readers.
4) When you want to show in the frontend a date range which already has a NULL end date, have an option to show the label from 3) instead of NULL.

I really don't think that an optional label and (1 checkbox plus its name) would require much effort in terms of coding.

While optional end date should be do-able quicker and easier.

If you have any suggestions please share them.
However, as it is now, with only the patch from #27, won't pass any usability or use experience tests.
I gave an example in #32.

dalra’s picture

@webchick

I saw your reply only after my previous post.

Please consider the events that have started in the past which do not yet have an end date (are still happening).
In this case, the users might enter the current day as the end date. That would be totally wrong, because it would provide a end date even though there is none yet.

Eg: How long have you been living in that city?

From 2016.10 to:
a) (Enter today's date - wrong choice - OR don't enter anything). This is confusing, leaves the user guessing what to do.
b) a checkbox: I'm still living here (of course the date is saved as NULL and we just show a label for NULL)

daften’s picture

@dalra, if there isn't much effort in terms of coding, I'd advice to make a patch :)

The coding part is often not the most difficult part, it's the usability and accessibility that are the difficult points sometimes. This also applies to the editor interface. Having the basic functionality of an optional end date (as it was there in D7) will be difficult enough. I'm not saying, and neither is @webchick, that your suggestions are invalid. I'd just advice you to put them in a different (follow-up) issue. This would help keep the basic discussion here clean and ONLY about optional end dates.

jonathanshaw’s picture

Those services don't actually let you pick an optional end date... they default to the current one, and if the from + to date match, they simply print the from date (this seems like very sensible behaviour for Drupal as well).

What Google calendar also does update the end date every time you update the start date, to keep the offset from the start date constant. This makes editing dates much less annoying.

Perhaps we don't need to allow the end date to be null, maybe it's OK to
1) simply default the initial end date to equal the start date (which we probably do already)
2) change the end date when the start date is changed to preserve the relationship
3) make the formatters show just the start date when the end date equals the start date

This makes it easy for a sitebuilder to setup a fairly good editorial experience.

Setting the end date to equal the start date seems an acceptable way of communicating "the idea of an end date doesn't apply here". Allowing NULL would be a way of allowing sitebuilders to say "this could have an end date but I don't know it"; but that doesn't seem like an 80% use case.

There's no doubt that if there was a situation where end dates were frequently not needed for a certain date field, it would be a better UX to have a checkbox like @dalra describes. But it's very easy to implement that in a contrib widget. The simplest possible version of @dalra's suggestion, which might be worth considering in core is to have a widget setting "Don't show end date input initially"; if checked it would hide the end date input using only css unless an "Add end date" link was toggled. But anyway, definitely this kind of UX refinement can definitely be postponed to a separate issue.

mpdonadio’s picture

#43, (1) and (2) are in #2863444: Discourage/make impossible to select a "to" date that is before the "from" date, which mostly works, but needs tests. (3) should be the default behavior in core already (see DateTimeRangeTrait::viewElements() and I know we have coverage in DateRangeFieldTest for this).

mpdonadio’s picture

Status: Needs work » Needs review
Issue tags: +Needs manual testing, +Needs update path, +Needs update path tests
FileSize
5.7 KB
2.75 KB

How about something like this? We still need something that works w/o JS, so #2863444: Discourage/make impossible to select a "to" date that is before the "from" date can only be an enhancement, not a solution.

mpdonadio’s picture

It is also worth mentioning that the only correct way to test this patch is to apply it it 8.6.x, then do a site install. Without the update path, the database schema / field definition will be wonky.

Birk’s picture

Added some extra logic to the #45 patch, that will require the end date, if optional_end_date is not selected.

Should the optional_end_date setting be moved to the field config instead of the field storage config, since it doesn't alter the storage anymore?

mpdonadio’s picture

Should the optional_end_date setting be moved to the field config instead of the field storage config, since it doesn't alter the storage anymore?

I'm included to keep it with storage so that the `hasData` check prevents it from being changed. Worried that if we get NULL values in storage and someone changes optional => FALSE, it will cause weirdness.

Birk’s picture

#48 Fair point.

Added an update hook, that changes the end_value column of existing fields.