Problem/Motivation

The daterange field should be placed into the "date_time" category.

Steps to reproduce

Install datetime_range module and you will see it creates its own category which leads to confusing UI.

Proposed resolution

This requires:

  1. Adding category = "date_time" to the annotation of DateRangeItem.php
  2. User interface text for the descriptions, to be added also to the annotation.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#28 Screenshot at Nov 29 12-54-19.png535.9 KBshweta__sharma
#25 3401464-nr-bot.txt85 bytesneeds-review-queue-bot
#23 3401464-22.patch1.87 KBajaypratapsingh
#21 issue-3401464-21.png415.47 KBsime
#17 Screenshot from 2023-11-15 16-23-19.png188.82 KBanushrikumari
#15 3401464-nr-bot.txt112 bytesneeds-review-queue-bot
#14 issue-3401464-date-range-should-fixes.patch705 bytesviren18febs
#12 Screenshot from 2023-11-15 13-01-32.png349.08 KBanushrikumari
#9 Screenshot from 2023-11-15 06-27-26.png112.15 KBanushrikumari

Issue fork drupal-3401464

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

sime created an issue. See original summary.

sime’s picture

sime
any/any
Location Melbourne
commented
Issue summary: View changes
cilefen’s picture

cilefen commented
Component: field system » datetime.module
Issue tags: -widget
rkoller’s picture

rkoller
he/him
Primary language German
Location Nürnberg, Germany
commented
Related issues: +#3370326: Refine labels and descriptions for field types

I've added #3370326: Refine labels and descriptions for field types as a related issue. I wonder if the entire point 2 from the proposed resolution could be done in the scope of the linked issue which is about refining the microcopy. That way the copy would be consistent when it is done in a single issue there?

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

sime’s picture

sime
any/any
Location Melbourne
commented

@rkoller I agree that refining microcopy would be best done on a dedicated issue.

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

anushrikumari opened merge request !5397

anushrikumari’s picture

anushrikumari commented
StatusFileSize
new Screenshot from 2023-11-15 06-27-26.png112.15 KB

Created MR to address point 1.

Added screenshot for review.

anushrikumari’s picture

anushrikumari commented
Status: Active » Needs review
sime’s picture

sime
any/any
Location Melbourne
commented
Status: Needs review » Needs work

Thanks for creating the MR with the initial change, we'll need the rest before the review can start.

anushrikumari’s picture

anushrikumari commented
StatusFileSize
new Screenshot from 2023-11-15 13-01-32.png349.08 KB

I've wrapped the category into @Translation. The description is already added for the field, see screenshot so do we have to add it anywhere else?
The description for the category is already updated at https://www.drupal.org/project/drupal/issues/3372097

sijumpk’s picture

sijumpk commented

@anushrikumari, I think you added
category = @Translation("Date and Time")
Just category machine name is needed. (its "date_time" for datetime field)
category = "date_time",
Otherwise it ends up showing inside a separate group.

viren18febs’s picture

viren18febs commented
Status: Needs work » Needs review
StatusFileSize
new issue-3401464-date-range-should-fixes.patch705 bytes

I have updated the category machine name & patch added, please review.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3401464-nr-bot.txt112 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

sime’s picture

sime
any/any
Location Melbourne
commented

It should be category = "date_time" you can compare it to the other date widgets.

Please leave "Needs work" until someone provides UI text that is similar to the other date widgets.

anushrikumari’s picture

anushrikumari commented
Status: Needs work » Needs review
StatusFileSize
new Screenshot from 2023-11-15 16-23-19.png188.82 KB

Added the description similar to the date field. Also updated the category to "date_time". Attached screenshot for the description.

sime’s picture

sime
any/any
Location Melbourne
commented
Status: Needs review » Needs work

Thank you. I think this will need something that describes why a site builder would use this field instead of simply using two date fields.

anushrikumari’s picture

anushrikumari commented
Status: Needs work » Needs review

Updated description that defines the module functionality. I would like some suggestions about the info that we should add/keep or omit from below -

  • Users can specify a start date and an end date for the content, defining the duration of the event or period.
  • With the date range field, users can easily search and filter content based on specific date criteria.
  • Field options
    • Date and Time: Stores date and time component.(E.g Thu, 11/02/2023 - 09:00 - Thu, 11/16/2023 - 17:00)
    • Date - Stores date only and time is set 12:00 hrs as default. (E.g Thu, 11/02/2023 - 12:00 - Thu, 11/16/2023 - 12:00)
    • All Day - Stores date and time, starting with 00:00 hrs of start date and 23:59 hrs of end date (Thu, 11/02/2023 - 00:00 - Thu, 11/16/2023 - 23:59)
  • Basic validation that date range provides
    • If the one date has value then other date cannot be NULL.
    • The end date is always greater than start date.
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Seems to have test failures in MR.

sime’s picture

sime
any/any
Location Melbourne
commented
Issue summary: View changes
Issue tags: +Needs usability review
StatusFileSize
new issue-3401464-21.png415.47 KB

Thank you @anushri19 this is good progress. I've screenshot the current version below for UX team review and added the tag. I expect the text is too verbose but it does seem to cover everything at this point.

I've looked at the gitlab test output and I can't work out what might be failing due to this patch, i'm not sure what I'm missing something. I have merged/rebased 11.x for good measure.

shows daterange field advice when adding to content type

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

Proposed a simplified text in the MR. 😊

ajaypratapsingh’s picture

ajaypratapsingh commented
Status: Needs work » Needs review
StatusFileSize
new 3401464-22.patch1.87 KB

I have tried to add points suggested by @lauriii please review

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

@ajaypratapsingh Thank you for working on this! This issue has a MR so it would be great if you could make the updates there 😊

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3401464-nr-bot.txt85 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

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

ankithashetty’s picture

ankithashetty
Location Karnataka, India
commented
Status: Needs work » Needs review

Rebased the MR and updated the description as suggested in #22, thanks!

shweta__sharma’s picture

shweta__sharma commented
StatusFileSize
new Screenshot at Nov 29 12-54-19.png535.9 KB

Tested MR-5397 and the changes have been updated perfectly. Attached screenshot for reference.
Thanks

image

shweta__sharma’s picture

shweta__sharma commented

Hi @lauriii
Some of my thoughts -
There is a full stop after the sentence in recent changes but with other field types there is no full stop in the description sentences so I think we need to add a full stop on the other field descriptions too for better experience. After the sentence is complete, there should be a full stop which is common.

https://www.stylemanual.gov.au/grammar-punctuation-and-conventions/punct....

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs review » Needs work

Maybe we could remove the full stops here and open another issue to decide if we should use full stop across all field type descriptions.

I also added a proposal to remove the field options since that isn't something we have for other field types, and it makes the description very lengthy.

shweta__sharma’s picture

shweta__sharma commented

Removed full stops and field options from the description as suggested in #30
Thanks

shweta__sharma’s picture

shweta__sharma commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs followup

Tagging for follow up per #30, hiding patches for clarity.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs usability review, -Needs followup

We already have a follow-up for refining the descriptions: #3370326: Refine labels and descriptions for field types. I added the full stop for bullet points as a consideration there.

This seems like a small enough issue to not require full on usability testing. Removing the tag and moving to RTBC since it would be nice to get this done before 10.2.0.

  • catch committed 29617467 on 10.2.x 
    Issue #3401464 by anushrikumari, sime, shweta__sharma, lauriii,...

  • catch committed 20c05b5c on 11.x 
    Issue #3401464 by anushrikumari, sime, shweta__sharma, lauriii,...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 10.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked to 10.2.x, thanks!

Status: Fixed » Closed (fixed)

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