Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UX] Fixed: Time picker in Date Pop-up calendar does not respect time format. #5703

Closed
yorkshire-pudding opened this issue Aug 2, 2022 · 13 comments · Fixed by backdrop/backdrop#4153

Comments

@yorkshire-pudding
Copy link
Member

yorkshire-pudding commented Aug 2, 2022

Description of the bug

Firstly, an AM/PM is shown when time is set to use G for hour (i.e. 24 hour with no leading zeroes). In #4064 @VasasA proposed a solution which works:
On
https://github.com/backdrop/backdrop/blob/8ce893d89106993c37ada5339355c9ead03bb8e8/core/modules/date/date.elements.inc#L1170

Change:

      'show24Hours' => strpos($element['#date_format'], 'H') !== FALSE ? TRUE : FALSE,

to:

'show24Hours' => (strpos($element['#date_format'], 'H') !== FALSE || strpos($element['#date_format'], 'G') !== FALSE) ? TRUE : FALSE,

but because the other issue in the ticket (dot after date) could not be solved, this has not been implemented.

Secondly, the tooltip has a time that doesn't respect the time format, e.g. 04:34PM and 05:34PM

I cannot work out where this is coming from.
Is Chrome autofill

Steps To Reproduce

To reproduce the behavior:

  1. Add a date field to content type
  2. Pick a date time format with G for time format
  3. Create a content and click into the time field

Actual behavior

Only 12 hour clock can be entered and PM is shown after
Tooltip shows in 12 hour format
chrome_sDc44RExoW

Expected behavior

24 hour time can be entered
Tooltip respects the time format

Additional information

Add any other information that could help, such as:

  • Backdrop CMS version: 1.22.2
  • Web server and its version: litespeed
  • Operating System and its version: n/a
  • Browser(s) and their versions: Chrome 103
@yorkshire-pudding
Copy link
Member Author

yorkshire-pudding commented Aug 2, 2022

I've added a PR that addresses this the first part. Credit for this should go to @VasasA . However, I cannot work out how to fix the tooltip, if that is too hard, we should go ahead with the PR as it stands

@indigoxela
Copy link
Member

indigoxela commented Aug 2, 2022

I've never seen a time tooltip on core date fields (with datepicker enabled). How do I have to setup the widget to get one?

Edit: ah, it ships with the module? 😀

@yorkshire-pudding
Copy link
Member Author

Did you find where it is? Does that mean its possible to disable it?

@indigoxela
Copy link
Member

Did you find where it is?

No... reading the core code - it should be there OOTB, right? Never saw one. Also not on D7...

Please enlighten me! 💡

@yorkshire-pudding
Copy link
Member Author

I don't know. I'll try in a different browser

@indigoxela
Copy link
Member

I tried in FF and Chrome on Linux. The script is loaded, but I never saw any time popup (tooltip).

@yorkshire-pudding
Copy link
Member Author

Was a Chrome autofill. It must have added whilst I was testing before I fixed the format.

@indigoxela
Copy link
Member

Nevertheless, G should still lead to a 24 hour format - which it currently doesn't (confirmed).

But... even with the patch applied, the format still has leading zeros. Probably because the datepicker widget is a bit opinionated when it comes to input formats.

@yorkshire-pudding
Copy link
Member Author

Yes, I couldn't find anywhere to change that, but it didn't bother me as much as not being able to enter 24 hour time. If there is a simple way to force it to use the format, then great, but if not, in my opinion, it shouldn't stop the 24 hour fix going in.

@yorkshire-pudding
Copy link
Member Author

😄 all automated tests have passed ✔️

@indigoxela
Copy link
Member

Seems like we agree, that the 24-hour format problem should get fixed - no matter if the leading zeros are there or not.

It works for me. 👍

@herbdool
Copy link

herbdool commented Aug 5, 2022

Looks good to me!

@quicksketch
Copy link
Member

Merged into 1.x and 1.22.x. Thanks folks!

@jenlampton jenlampton changed the title [UX] Time picker in Date Pop-up calendar does not respect time format [UX] Fixed: Time picker in Date Pop-up calendar does not respect time format. Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants