Problem/Motivation

The field labels for the daterange field widgets are ambiguous. They simple appears as "Start" and "End".

Proposed resolution

Update the labels to "Start date" and "End date".

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#15 2796145-15.patch1.01 KBmpdonadio
#6 daterange_field_labels-2796145-6.patch1.01 KBkunalkursija
#2 2796145-02.patch1.09 KBmpdonadio
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mpdonadio created an issue. See original summary.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio at DiD Agency commented
Status: Active » Needs review
FileSize
2796145-02.patch1.09 KB

Not sure what else needs to be done here.

hass’s picture

hass CreditAttribution: hass commented
Status: Needs review » Needs work

This should better named "Start date" and "End date". I would also translate it this way in German. We may need no context than.

dishabhadra’s picture

dishabhadra CreditAttribution: dishabhadra at Blisstering Solutions, Iksula commented
Assigned: Unassigned » dishabhadra
kunalkursija’s picture

kunalkursija CreditAttribution: kunalkursija at Iksula, Blisstering Solutions commented
Assigned: dishabhadra » kunalkursija
kunalkursija’s picture

kunalkursija CreditAttribution: kunalkursija at Iksula, Blisstering Solutions commented
Status: Needs work » Needs review
FileSize
daterange_field_labels-2796145-6.patch1.01 KB

Adding revised patch.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio at DiD Agency commented 
+++ b/core/modules/datetime_range/src/Plugin/Field/FieldWidget/DateRangeWidgetBase.php
@@ -20,10 +20,10 @@ class DateRangeWidgetBase extends DateTimeWidgetBase {
-    $element['value']['#title'] = $this->t('Start');
+    $element['value']['#title'] = $this->t('Start date');
 
     $element['end_value'] = [
-      '#title' => $this->t('End'),
+      '#title' => $this->t('End date'),
     ] + $element['value'];

The scope of the issue was just to add the translation context, not to change the actual labels (which would not(?) be 8.2.x eligible, not 100% sure if this policy also applies to experimental modules). There are also hidden labels "Date" and "Time" on the Element, so I am not sure if this change really makes sense from a UX/A11Y perspective.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented

Agree that changing the label is out of scope for this issue--feel free to open a new one (which would need UX/A11Y evaluation) for that.

I think #2 is good to go unless there's more to be done? Not sure if translation contexts are testable or not?

hass’s picture

hass CreditAttribution: hass commented

Context does not scale on l.d.o. It should not used whenever possible.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented
Status: Needs review » Postponed (maintainer needs more info)

This issue was specific to adding a translation context, but the feedback indicates these should not be used. What's the next step here?

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio at DiD Agency commented
Assigned: kunalkursija » Unassigned
Status: Postponed (maintainer needs more info) » Needs review

Pinging @GaborHojtsy for an opinion here.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented
Assigned: Unassigned » Gábor Hojtsy

Looks like #11 intended this to be assigned to Gabor.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Title: Daterange field labels should provide a translation context » Daterange field labels not specific enough
Assigned: Gábor Hojtsy » Unassigned
Status: Needs review » Needs work

I personally think changing the string themselves would be preferable. The granularity of a 'Date range field label' context sounds too much to me. I also believe we can change strings in new minor releases (eg. 8.2 to 8.3).

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Issue tags: +Needs issue summary update
mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio at DiD Agency commented
Version: 8.2.x-dev » 8.3.x-dev
Issue summary: View changes
Status: Needs work » Needs review
FileSize
2796145-15.patch1.01 KB

Thanks Gábor.

This is just the patch in #6. It applies to both 8.2.x and 8.3.x. Since this is an experimental module, I think we can apply it to both, but I bumped up the version to 8.3.x and the committers can decide.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs issue summary update

This looks good. IS was already updated as part of #15, so removing that tag.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 15: 2796145-15.patch, failed testing.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Needs work » Reviewed & tested by the community

Unrelated fails in aggregator tests.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 15: 2796145-15.patch, failed testing.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Needs work » Reviewed & tested by the community
Issue tags: +sprint

Hum, now failing on another unrelated CommentHalJsonBasicAuthTest...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Issue tags: +String change in 8.3.0
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed f3baea1 and pushed to 8.3.x. Thanks!

  • alexpott committed f3baea1 on 8.3.x 
    Issue #2796145 by mpdonadio, kunal.kursija, Gábor Hojtsy, jhedstrom,...
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Issue tags: -sprint

Yay, thanks!

  • alexpott committed f3baea1 on 8.4.x 
    Issue #2796145 by mpdonadio, kunal.kursija, Gábor Hojtsy, jhedstrom,...

  • alexpott committed f3baea1 on 8.4.x 
    Issue #2796145 by mpdonadio, kunal.kursija, Gábor Hojtsy, jhedstrom,...

Status: Fixed » Closed (fixed)

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