Hi, I'm using Scheduler module version 8.x-1.0-rc2 on Drupal 8.3.7.
When I check the 'Allow users to enter only a date and provide a default time' option and try to create a new content which has scheduler configured (in my case request just an unpublish date), the time field is not been populated with the default time, however, you can see in the message that the default time it has been set:
Enter a date. The time part is optional. The default time is 00:00:01.
(been 00:00:01 my default test time).
Trying to debug this I got to the method /src/Plugin/Field/FieldWidget/TimestampDatetimeNoDefaultWidget->valueCallback, the definition for this method is
public static function valueCallback(&$element, $input, FormStateInterface $form_state)
however, the $input variable is empty, that's why it doesn't pass the validations and it's never set. If I try to set the default time value for this field I also need to define a default date value which should be the value entered by the user, but since $input is empty I have no place to grab this value from (maybe from $form_state? but this doesn't seem to be right).
I also test the module in simplytest.me and got the same results. Form this point I'm not sure how to proceed to keep debugging this.
How to reproduce:
- Check the 'Allow users to enter only a date and provide a default time' option and define a default time.
- Create a new content which the scheduler option set.
Comment | File | Size | Author |
---|
Comments
Comment #2
eworwa CreditAttribution: eworwa as a volunteer commentedComment #3
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedHi eworwa,
Sorry to hear you are having a problem. However, I cannot replicate it. Using 8.3 and 8.4, with Chrome and Firefox, it works OK. Also our automated testing passes the 'default time' test. So in principle it is working. Now, we need to discover what combination of things is causing your problem.
Jonathan
Comment #4
giordy CreditAttribution: giordy commentedI have the same problem.
Thanks
Comment #5
gsquirrelAlso have same problem it just says "please fill in this field".
Comment #6
gsquirrelI discovered that this for me is connected with the field being "required" in the settings for content type. If I untick "Require scheduled unpublishing" then it works and sets a default when only a date is selected.
When the field is required the validation prevents you from submitting the form without both a date and a time.
Comment #7
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedAh, thanks gsquirrel, that is useful information. Yes I can replicate this error now. It is strange because I am sure this was working before - it is such a basic feature it would be surprising for this bug not to have been found before. I know that browsers implement the HTML5 date widget differently and developers now have reduced access to manipulate the data. I recall trying to set the default time into the field when the form is first shown, but I think it failed because just entering a time was not allowed.
I will try again to find a solution for this.
Comment #8
RebeccaPossibleAny update on this issue? I still get the required message even though time is optional.
Comment #9
baddeed CreditAttribution: baddeed commentedPlease fix it. It will be my life saver.
Comment #10
Stockticker CreditAttribution: Stockticker as a volunteer and at Internetdevels, Drupal Ukraine Community commentedComment #11
Stockticker CreditAttribution: Stockticker as a volunteer and at Internetdevels, Drupal Ukraine Community commentedI've got the same issue on one of my projects and decided that it would be a good idea to populate default time on a frontend if those requirements are met:
Should work both for vertical tabs and a fieldset.
Please, take a look and test.
Comment #12
Stockticker CreditAttribution: Stockticker as a volunteer and at Internetdevels, Drupal Ukraine Community commentedComment #13
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedThanks Stockticker. Yes, using jquery to pre-populate the field might work. Before HTML5 we had the ability to set the default time, but when moving to Drupal8 we lost that. But your idea is good.
Comment #14
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedFollowig my work on #3126644: Create SchedulerSetupTrait for common test start-up processing this new javascript can have test coverage. Here's a patch with just the new test, not the new functionality so this should fail.
Comment #15
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedTests ran OK locally.
But
WebdriverTestBase
should beWebDriverTestBase
so this could have caused the 'class not found' error.Comment #16
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedThe new tests were not run. Need to add to the drupalci.yml:
Comment #18
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedThat's good, the tests run and fail as expected. Patch #18 includes the new js file and functionality as is Stockticker's patch #11
Comment #20
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedAh, that's the problem. Locally the tests run fine, but my Chromedriver calendar pop-up is set to accept format d/m/Y which is the default for my location (UK). I think the drupal.org testsbot, because it runs in Australia/Sydney timezone, is expecting m/d/Y.
For a user in Sydney timezone
1607023800 = Fri 04-Dec-2020 - 06:30:00 = 04/12/2020 = actual
1586896200 = Wed 15-Apr-2020 - 06:30:00 - 15/04/2020 = expected
I have now switched the format in the javascript test to be m/d/Y, and locally I now get the failures
Failed asserting that 1607023800 matches expected 1586896200.
which is exactly the error on d.o. Patch #20 may pass on drupal.org but clearly the test needs work as it should not depend on what location the browser thinks it is in.Comment #21
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedI've added a way to work out which HTML5 datepicker format is being used in the test. These run sucessfully locally, which uses d/m/Y. Testing on drupal.org needs m/d/Y as we discovered above.
Comment #23
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedAdd screen-grab to try to find out why the node save failed.
Comment #25
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedScreen shot implied that the time format 'hh:mm:ss' was not enough and it need 'am' or 'pm'. This might be ok, as adding the extra text locally does not cause an input/form validation error.
Comment #27
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedCan't test this locally, hence all these attempts on d.o. Make it qucker (and use fewer resources) by only running the javascript.
Comment #28
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedNow that we have tests running properly, here's a patch without the new js library, to prove that it fails for this and not the datepicker format problem.
Comment #30
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedThis should be the final test.
Comment #33
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedMinor tweak to check for
element.val() === ""
instead of!element.val()
because on the first loop the value is blank (so we set it) but on subsequent loops the value is 'undefined'. So testing for blank feels a better way to do it.Thank you @Stockticker for the original patch (which pretty much made it entirely unchanged through to commit) and thanks @eworwa for raising the issue and @gsquirrel for help in narrowing down the cause of the problem.
Committed and fixed.