Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
When you add a datelist field, the Years available range from 1900 to 2050.
For many uses, that's unnecessarily broad.
It'd be good to be able to configure the range in the manage form display interface.
E.g. to +/- 3 years, or min 2010 and max 2020, etc.
I think Drupal 7 had this feature.
Comment | File | Size | Author |
---|---|---|---|
#61 | drupal-n2836054-61.patch | 20.05 KB | DamienMcKenna |
#61 | drupal-n2836054-61.interdiff.txt | 1.1 KB | DamienMcKenna |
#58 | 2836054-nr-bot.txt | 144 bytes | needs-review-queue-bot |
#51 | interdiff_50-51.txt | 3.31 KB | raman.b |
#51 | 2836054-51.patch | 20.44 KB | raman.b |
Comments
Comment #3
larowlangiddyup
Comment #4
larowlanThis works for me
Comment #5
jibranThis is an experimental module so no need for the upgrade path and upgrade path tests.
Comment #6
jhedstromA few questions/comments here.
Why is this only specific to the daterange field? Both normal datetime and date range fields support select list entry. Also, this is currently applied as a setting regardless of the form widget selected. It is only applicable to select list entry I think (unless it is meant to validate/constrain dates between those years, in which case it needs work since that bit isn't working...).
Comment #7
mpdonadioAlso, we already have a Datelist form element, which that widget is using. Can't we just expose the setting and then stuff it into #date_year_range key in DateRangeDatelistWidget::formElement() and DateTimeDatelistWidget::formElement().
Since PluginSettingsBase::getSetting() merges in defaults, I don't think either would need an upgrade path and upgrade path tests, just a cache clear.
Comment #8
larowlanThat's what the patch does, the YearRange form element is just for the settings form, borrowed from the approach in date_api in D7.
I agree, this can go up one layer further to the single date field, but that'd be in the non-experimental module.
Comment #9
jibran> I agree, this can go up one layer further to the single date field, but that'd be in the non-experimental module.
Let's do that. I believe we can add new features to any module in a minor release.
Comment #10
mpdonadioI just re-read this. Haven't played with it applied, but I like the better UX of the form element approach rather than a simple text field.
And yeah, this can go into the datetime version, too. As long as the datetime version doesn't depend on the daterange one, they this is a valid new feature for 8.4.x: it adds a new admin feature to an existing form in a non-disruptive way. A select with 150 things in it is a UX WTF anyway.
static
I don't think the setting should be on the base class, just the list widget.
Extra indent.
We can just use the ->save() w/o an intermediate local variable.
Can use [].
Looks like there is no coverage of the new form element on the manage display or did I miss something?
Comment #11
jibranComment #12
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commentedComment #13
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commentedRerolled the patch.
Comment #14
jofitz CreditAttribution: jofitz at ComputerMinds commentedFixed minor coding standards issues.
Removed Needs Reroll tag.
Comment #15
mpdonadioDoesn't look like #10 has been addressed.
Comment #16
jofitz CreditAttribution: jofitz at ComputerMinds commentedSorry, my mistake. I didn't read properly and assumed they had all been address in #13.
Comment #17
jofitz CreditAttribution: jofitz at ComputerMinds commentedComment #19
jofitz CreditAttribution: jofitz at ComputerMinds commentedAdd defaultSettings() to DateRangeDefaultWidget too.
Comment #20
mpdonadioDid some manual testing.
Currently doesn't work on datetime fields. Let's make it work there, too.
Like the way this works on daterange. Big UX win.
Comment #22
jofitz CreditAttribution: jofitz at ComputerMinds commentedPatch in #19 no longer applies cleanly. Re-rolled.
Comment #23
jofitz CreditAttribution: jofitz at ComputerMinds commentedAdded to datetime fields in the same way it is on daterange fields.
Comment #25
mpdonadioI think we need to move this to DateTimeWidgetBase.
Need to look at this applied, but this looks like it could be out of scope? But if we do the #1 bit, then this is probably needed.
This def needs to move to DateTimeWidgetBase() so it is available in all of the Datetime widgets.
This can't be in datetime_range. At a min, it needs to be in datetime, but possibly in with the core Elements since there could be value outside these modules.
?
See above bit about moving to the base class.
Need to add coverage for the datetime widgets and mirror the test in datetime module for these settings.
Comment #26
jofitz CreditAttribution: jofitz at ComputerMinds commentedAlso fixed test failure and coding standards error.
Comment #28
jofitz CreditAttribution: jofitz at ComputerMinds commentedCorrected error in new test.
Removed unused use statement.
Comment #29
jhedstromShould we add a kernel or unit test specific to this new render element? I was trying to find a specific one for the existing data render element, but could not find one, however many render elements have dedicated unit tests. These public methods seem pretty easy to unit test.
Let's add at least one test that deals with relative dates to ensure those are working (eg, -3:+3).
Comment #30
mpdonadioAgree with needing a little more test coverage.
#2 should test 1978:2017, -3:2017, 1978:+3, -3:+3, ie all of the possibilities.
Comment #31
Anas_maw CreditAttribution: Anas_maw as a volunteer commentedNot working if you don't give the field default value.
Comment #32
jofitz CreditAttribution: jofitz at ComputerMinds commentedComment #33
Anas_maw CreditAttribution: Anas_maw as a volunteer commentedHello is it possible to make this work if we use 'now' date for example?
Also try to enter start day +1 and the end date +10 for example, this is not working.
Comment #34
jofitz CreditAttribution: jofitz at ComputerMinds commented@Anas_maw Rather than 'now' you can use '+0' (or perhaps open a follow-up issue).
What is not working with '+1:+10'?
Comment #36
jofitz CreditAttribution: jofitz at ComputerMinds commentedComment #37
Josue2591 CreditAttribution: Josue2591 at Manatí commentedTested in 8.6.x. Works.
Comment #38
mpdonadioWe need to add a check so that the start year <= the end year, and test coverage for that.
Comment #39
jibranNow that
datetime
anddatetime_range
are not experimental anymore we need an upgrade path and upgrade path test as well.Comment #40
mpdonadioSince these are just widget settings with default values, don’t we just need an empty post update to trigger a rebuild?
Comment #41
jibranAs we are adding a new key to the
datelist
anddefault
formatters we need to update the config for the existing field formatters with the new key and default values.Comment #42
iyyappan.govindHi Team,
I love this post. I have same issue, I just used the form_alter to restrict the date range. So my date range is 1990 to current year. It works fine but I need the descending date sort order, means 2018, 2017, ...1990. Is this possible via form alter? It would be great if we implement the sort order in UI if it is not in date form api.
Thank you.
Comment #46
Rob230 CreditAttribution: Rob230 as a volunteer and at CTI Digital commentedWould love this feature as well. It's not very useful for example to have a date of birth field where you can choose 2050 as the year.
Comment #47
manuel.adanJust 8.8/8.9 re-roll with no additional changes.
Comment #49
mpp CreditAttribution: mpp at AmeXio for District09 commentedThe patch in #47 has an issue with the namespace for t()
MachineNameTest.php also defines Drupal\Core\Render\Element\t, not t() so function_exists will return false (see https://www.php.net/manual/en/function.function-exists.php#117916) but will trigger this error:
Comment #50
raman.b CreditAttribution: raman.b at OpenSense Labs commentedUpdating Functional tests & addressing #49
Comment #51
raman.b CreditAttribution: raman.b at OpenSense Labs commentedComment #54
kim.pepperComment #58
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #59
DamienMcKennaRerolled, applies cleanly against 9.5.x and 10.1.x.
Comment #60
DamienMcKennaAm working on the fixes.
Comment #61
DamienMcKennaThis will hopefully fix the problems; I accidentally lost the t() function change in YearRange.php.
Comment #62
DamienMcKennaI don't understand why the t() change is required in the first place? But it's causing the test to fail.
Comment #64
DamienMcKenna