fix(timepicker): respect meridian setting when entring hours

[pkozlowski-opensource]

Fixes #1631

[deeg]

LGTM

[pkozlowski-opensource]

@ultimatedion not sure I understand your comment, for me things work as expected. To start with let's see some things straight. According to https://en.wikipedia.org/wiki/12-hour_clock we should interpret:

• 12:00 AM as midnight (00:00 in the 24h format)
• 12:00 PM as noon (12:00 in the 24h format)

Given the above we've got 2 cases when users enters 12:

• AM is selected - in this case entering 12 will result in 12:00 AM (00:00)
• PM is selected - in this case entering 12 will result in 12:00 PM (12:00)

For me this is proper behaviour. Could you please clarify what exactly what you mean by

If you type 12, it will select PM which is possibly not right.

The idea of limiting possible values to enter to 1-12 is another topic - feel free to open an issue for this.

I'm going to hold of merging of this PR for a bit to give you a chance to comment (but won't delay it for long). But please, be more precise as I'm really not clear at this point what is troubling you in the code of this PR.

[ultimatedion]

@pkozlowski-opensource You are right. My analysis of your fix was wrong. I had in mind that the isPM was related to the new value. I'm sorry for the troubles.

I agree with you for the first case.
But are you sure about the 2nd case?

•PM is selected - in this case entering 12 will result in 12:00 PM (12:00)

• Model hour = 12 (12:00 PM on screen)
• user enters 12

According to this line:
``this.model.updateHour(toInteger(newVal) + (this.meridian && isPM ? 12 : 0));

newVal(12) + 12 = 24 which will be converted back to 0
Model hour result will be 0 (12:00 AM on screen)

This might not be an issue if the updateHour isn't triggered if we type the same value.

I cannot test it at the moment.
I'm truly sorry if I'm wrong again.

[ExFlo]

Hi

Actually I was going to say the same thing: entering 12 with PM on screen will result in 0 AM I think.
Maybe it is missing here some test on the limit value just to be sure of what we want and have.
Seems like also entering 14 when in PM case will result in 2 AM at the end, right? Maybe this is what we want but in this case I suggest to add a test for this behavior too
BR

[pkozlowski-opensource]

@ultimatedion @ExFlo you are right, there is an issue for this case:

PM is selected - in this case entering 12 will result in 12:00 PM (12:00)

I will add test for the noon / midnight cases to explicitly say what is the expected behaviour in those cases.

Thnx for the heads up and review, much appreciated!

[pkozlowski-opensource]

@ultimatedion @ExFlo I've added explicit tests for the 12:00 AM / PM case and amended the logic. Please take another look. Thnx!