Hide seconds when entering scheduled date/time

This is really useful, thank you mr.baileys. I will take a closer look when I get time. My hunch is that we should keep it flexible with the option to show/hide, because there is a possibility that users will want to unpublish then republish within the same minute - to allow ideas such as #2940996-5: Add re-publish date

Makes definitely sense! It is quite annoying to type in seconds every time you schedule a node.

I noticed on manual testing that where a pre-existing node already has a seconds value in the scheduler field then the seconds are not hidden but are greyed out and not editable. This is OK though, just something for the site admin to be aware of. Likewise if the default time has a seconds value then new nodes get this value, and the seconds are greyed but not hidden. I will add a comment in the admin form to warn about this.

But before we add a new feature we also need test coverage. Here is a patch which adds to SchedulerAdminSettingsTest to check the admin form and SchedulerDefaultTimeTest to check the step value in the edit form. This patch should fail testing.

Here's the full patch with changed code and tests. Also interdiff for minor changes to the original (comments, moving config items into alphabetical order, extra description on admin form)

Is the patch to 8.x-1.x-dev now part of the full release 8.x-1.0 or should it still be applied?

Hi julius95,
Patches and fixes are never added to an existing release, they are first added (committed) to the -dev branch, then at some future time the dev branch is tagged with a new release number (eg 8.x-1.1) and that becomes the new version.

This patch has not yet been committed to dev - you would see an automated comment in this issue when it does get committed. So if you want to use this feature now, you will have to apply the patch. It probably could be applied to your own 1.0 release I expect, if you don't want to use the dev version on your site.

Jonathan

Re-rolled patch from #6, because I had difficulties to apply the patch via composer (using cweagans/composer-patches).

Hello!
Tested patch from # 9 and it worked for me, but when I'm editing a node that has a publishing date with seconds after checking the checkbox on config page, the seconds continue to appear, and it is disabled, even after I save the node.
Steps:

• With the checkbox unchecked, add a new Article with a unpublishing date with seconds
• Check the "Hide seconds when scheduling." checkbox on config page
• Edit the previous added node, note that the seconds is shown disabled

I don't know what is the best approach in this case, but shouldn't the seconds be cleared ?
If this is the correct behavior, then we can change the issue to RTBC.

Tested with Drupal 9.1.8 and Scheduler 8.x-1.x-dev.

Hi @hmendes,
Thanks for testing this, and for giving feedback. The first thing I noticed is that unfortunately @osopolar did not re-roll patch #6, but started from scratch with the initial patch from the original issue summary. That meant that the tests got dropped, amongst other minor changes I had made. These were shown in the interdiff I added to comment #6 above.

So now to get back to the actual proper code to test I have re-rolled patch #6 and committed it all to an issue branch and opened a merge request. What this means is that if you do not want to clone the issue branch, you will always have read access to a complete patch for the issue at the url https://git.drupalcode.org/project/scheduler/-/merge_requests/5.diff (which is also the url behind the 'plain diff' link in the MR box)

Using the MR I have also added a tugboat config file, which means we should get a live preview of a plain Drupal site with Scheduler installed and the patch applied. This allows online testing which will be good to see if your local results match the test site.

I can't reproduce what went wrong on my patch re-roll, but anyway thank you @jonathan1055 for the new one.

No worries, osopolar. The new live preview testing site is good, so it was worth starting off with a new issue branch.

@hmendes I have noticed some variations in functionality between different browsers. Unfortunately, the html5 date elements behave differently. When there is no date the form correctly does not show the seconds, but when a date has already been set, on Chrome the seconds are shown disabled, on Firefox the seconds are not disabed. On Safari you cannot enter seconds directly but you can use the picker to alter the value. In all cases the html validation prevents the seconds being changed, and you get various messages indicating how to enter a valid time.

There are some improvements I can make, such as setting the seconds to zero when editing content where it had a prior value with non-zero seconds. I can also improve the help text and admin settings, to use just hh:mm when seconds are hidden. This won't solve all the problems, but will be an improvement.

Hello @jonathan1055,
When on Firefox, the form validation message is "Please select a valid value. The two nearest valid values are h:m:s and h:m:s", isn't there a way to show a more specific drupal error message instead of this form error message?
This error was caused by not following a configuration made on Drupal, so wouldn't it be a better idea to show a drupal error, telling the user that he can't change the seconds? I also think it would be a good idea to set the seconds to 0, as it would not be used.

Hi, Thanks for the feedback.

"Please select a valid value. The two nearest valid values are h:m:s and h:m:s"

Yes, I found exactly that message when testing on Firefox. Chrome shows the seconds, greyed and disabled, but does allow the time picker to be used to select and change the seconds. However, the browser validation is similar and requires that the changed seconds value gets put back to what it was before.

isn't there a way to show a more specific drupal error message instead of this form error message?

Actually, I don't think there is, because those validation messages are produced by the browser before Drupal sees the form submission.

I also think it would be a good idea to set the seconds to 0, as it would not be used.

Yes you mentioned that before and I agree. I wanted to get the existing patch from #6 re-rolled with no changes first, and get the live preview via tugboat. I have the chnages locally to set the seconds to zero, and will push that soon.

It works fine when creating a new node, but on editing an existing one, even if the seconds are zero, they still get shown in the form. I am working on a method to really hide the seconds, so that the value cannot be altered, and we avoid the browser validation error completely.

@hmendes, I have made the above changes which should fix the things we found. Let me know how you get on with testing, either via the MR5 diff patch or the tugboat preview site

Hi @jonathan1055,
Tested your MR on Chrome and Firefox, and it's working perfectly for me. Thanks.
Changing to RTBC.

Thanks hmendes for testing, and thanks to mr.baileys for requesting the feature in the first place.
Merged and committed.

This was missing an update hook to set the new default config, which resulted in the default time being invalid, which caused HTML5 validation to prevent form submission

Saving the config form is enough to fix it, but might be worth a followup?

Actually nevermind, 2 years has passed and no-one else has had the issue, ignore me

Thanks for the comment. I checked and 8.x-1.4 was released on 19 July 2021, so it did have this commit in. Therefore it has been out for over two years. I'm happy to fix it if you want? Or is it too late now?

Nah it's not worth worrying about now, sorry for the noise