Switching between AM and PM

[morgangit]

This is almost perfect! But a serious issue I've found is that if you choose a time within range, you can then switch from AM to PM (or vice versa) to take the time out of range.

The simplest way to deal with this might be to disable AM or PM switching when doing so would take whatever time is currently displaying out of range.

[Agranom]

Could you reproduce this issue on https://stackblitz.com/ please? Because disabled time should recount after switching

[morgangit]

Actually if you go to your demo https://agranom.github.io/ngx-material-timepicker/ and click on the "Time range from 12:10 am to 08:11 pm" example, you'll see that if you select 9.00am and then switch to pm, it'll output 9pm although it's out of range.

[Agranom]

Okey, I see and I'll fix it

[morgangit]

Great - thanks for your quick responses. The only other thing that's a little odd (on the same time range demo) is that when the user clicks on a time that is out of range and greyed out, rather than simply being unselectable, it selects the soones possible time, or if a time has been entered, the previous valid time. It would be far more useful if the invalid times were simply not selectable (i.e, nothing happens when you click/touch them or a snack bar message says something like "You can't select this time".)

[morgangit]

Hi Agranom - was wondering if you're still working on these issues? No worries if not - just would be good to know as we were going to use it for a project but time is a factor.

[Agranom]

Hi. Yes, I've already fix it, but lack of time to finish unit tests. But nevertheless, I'll publish fixes this weekend, sorry for this

[morgangit]

Oh great. Thank you! Look forward to trying it when you publish the fixes.

[Agranom]

It's ready. Update to the latest version and try

[morgangit]

Hmm - I tested it. Not a great solution to the problem from a user perspective. I mean it allows you to select an invalid time, then outputs no time and logs to the console. I assumed the goal of this clock was to prevent the user from entering invalid times according the minimum or maximum (or both) criteria set. Allowing the user to select an invalid time , press OK and then outputting no time and a console error could be achieved without changing the interface at all. The optimal and obvious solution would be to disable actions that would make whatever time is currently displaying invalid. So disabling AM or PM switching and as I mentioned in my second comment, disabling selection of greyed out times. As I mentioned in my second comment, allowing the user to select greyed out times and then jumping to some other time that IS valid is not a viable solution in my opinion. Is it difficult to make greyed out times simply unselectable? Or disable AM PM switching?

[Agranom]

Do you mean to avoid switching between am pm if the whole diale is disabled?

[morgangit]

No - I mean disabling switching when switching would take whatever time is currently displayed out of range (or below the minimum or above the maximum in the other cases). I assume this could be done by attaching a condition to the AM and PM actions. So let's say there is a time displaying in AM (AM is selected), if the user tries to switch to PM, the condition would be: IF [(PM equivalent of current display time)<(min time) or (PM equivalent of current display time)> max time)] , disable PM action. You could have snack bar error "Invalid time" or "Out of permitted range" or whatever when the user tries to switch in this case to avoid frustration.

And vice versa for switching to AM from PM. This way, you can open the clock on the minimum time and simply make it impossible for the user to create an invalid time in the display, avoiding the scenario where the user enters and invalid time, gets a console error and then has to re-open the clock or start again - which is the same reason why I think it's better if greyed out times are simply unselectable.

[Agranom]

I see, but now user cannot select disabled time. It could be the good idea to avoid switching from AM to PM if there are no available time in PM (for example). But if there is at least one available time, user can switch and select only that time.

If make it in your way I'm afraid that user might not understand why he cannot switch the period

[morgangit]

I know what you mean, but with my suggested approach the user can switch if the currently displayed time would be valid after the switch - regards the user not understanding, a snack bar error message could solve that. It could even be more explicit "This time would not be valid in PM" or something.

But I take your point - I think the alternative then is for switching to jump to a valid time in the other meridiem. Currently it's still possible to switch meridiems and end up with an invalid time displaying and then press OK. So yes, I think that a reasonable alternative to my suggestion is to have the display time change to the minimum valid time of whatever meridiem the user switches to. Then, once again I think making selection of greyed out times impossible would make this a really useful clock.

[Agranom]

Currently it's still possible to switch meridiems and end up with an invalid time displaying and then press OK

It's already impossible. If user switch a period and time is not available, it selects the closest available one.

[morgangit]

Oh - sorry - that's great if so. I was testing on your demo - perhaps that isn't updated yet. What do you think about making greyed out times unclickable?

[Agranom]

To be honest, I've been thinking about it for a long time, but another issues interrupted me. I hope to do it in the next release

[Agranom]

I was testing on your demo - perhaps that isn't updated yet

By the way, update to 2.6.3, there're some of bug fixes

[morgangit]

Ok thanks - I'll update and check it out now. Looking forward to next release with greyed out times unselectable!

[morgangit]

Hi - I'm not sure if you didn't push changes or something but the latest version doesn't work as you describe above. If I switch meridiem the time remains the same even if the new time is invalid. There is no jumping to a valid time. Then you can hit 'Ok' and no time is output - just a console error - just like before.

[Agranom]

Follow the link https://i.imgur.com/7a1POxD.gifv please. I hosted there gif where it works right

[morgangit]

Apologies - I was testing the time range scenario - not the minimum time only one.

[morgangit]

Hi Agranom - I've realised that the issue I had in testing remains in all cases - it just applies to switching when minutes are selected. In the gif above you switch to AM when the hour is selected - if you continue and select minutes (starting at 1:10pm, say, with the '10' selected) and then switch back to AM, you'll find that you can have 1:10 AM, although the minimum is 3AM. I guess a similar fix needs to be applied for when switching from minutes.

[Agranom]

Hi, okay. I hope I'll fix it today

[Agranom]

I published 2.6.4, where I fixed bug with clicking on disabled hours.
Unfortunately I run out of time to fix disable switching between periods this week. I hope I'll do it the next week

[morgangit]

Ok thanks. I don't think you need to disable switching between periods. I think the approach you half-implemented where you don't disable switching but the time jumps to the earliest possible time on switching is a fine approach if you complete it. The problem is just that when minutes are selected, this doesn't work right now. Completing your approach I guess would mean that even if minutes are selected, the hour would have to jump to the earliest possible valid hour given the restrictions entered.

[morgangit]

Hi Agranom - any plan to release with a fix for the AM-PM switching issue that remains when minutes are selected?

[Agranom]

Hi. Could you please tell me steps to reproduce it, because I'm a little bit confused )

[morgangit]

Ok - an easy way to reproduce it using your demo. Go to the time range example (12.10am to 8.11pm) and switch to AM and pick 9, then 20 in minutes to get 9.20am. Now switch to PM and you get 9.20pm (out of range!) and you can press OK to get no output - error logged to console.

Or quicker:

1. Click AM
2. Click 9
3. Click pm
   You get 9pm and can click OK.

This clock would be most useful if you simply can't get a time that's out of range to display. So in this case when you have 9.20AM displaying, when you go to switch to PM it should jump to 8.11pm as that's the nearest possible valid time. This is what happens if you switch period with hour selected. Either that or it could jump to 7.20PM or even 12.20PM - just moving the hour somewhere to make it valid. Any of those solutions would work well.

[Agranom]

Hmm, I see. However, I guess that it's easier just to avoid user switching between periods and show some kind of warn or error message.

[morgangit]

Yes that would be another approach - and easier to implement I imagine. But I think it would be strange to have half-implemented an approach of jumping to a valid time when hours are selected and half-implemented an approach where switching is disabled if minutes happen to be selected. It would be inconsistent UX. When switching would cause an invalid time to display, either you can't switch - with a snack bar error showing ("Switching periods would take this time out of range" or something) , or switching should jump to a valid time in the period you switched to. Either would work fine - but not both. One strategy when hour is selected and another when minutes are selected would be quite bad.

[Agranom]

I've implemented avoiding switching between periods thing for now, because I have some important issues to resolve. Once I accomplish them, I'll try to implement it in your way.

[morgangit]

Hey Agranom - ok, thanks - by the way your snack bar error should be 'There IS no available time for this period' but I think that's a little unclear (which period? Also it implies that the entire period is out of range). I'd suggest changing the error to something like 'Current time would be invalid in this period'. Another issue is that the error never disappears! It should disappear after 3 seconds or so.

[Agranom]

Ops, thanks for mention it, I'll change.

[morgangit]

Hey - please let me know when the snack bar error problem I mentioned is fixed!

[Agranom]

already published.

[morgangit]

Great. Thank you! Do you want me to close this issue or do you still plan to implement the consistent solution of changing to a valid time on switching (the way it works when hour is selected)?

[Agranom]

I close it for now and once I finish other issues I'll try to implement your idea