maxkeppeler / sheets

⭐ ‎‎‎‏‏‎ ‎Offers a range of beautiful sheets (dialogs & bottom sheets) for quick use in your project. Includes many ways to customize sheets.
https://maxkeppeler.github.io/sheets/
Apache License 2.0
921 stars 77 forks source link

Clock bottom-sheet giving wrong time #7

Closed noyize closed 3 years ago

noyize commented 3 years ago

The result time is not same as of the selection in clock bottom-sheet. Also when using disableTimeline(TimeLine.PAST) in Calendar bottom-sheets the dates are still selectable

maxkeppeler commented 3 years ago

Right now I'm at work but will check those things out as soon as I'm at home. However, when I open the ClockTimeSheet in the sample, the result time it returns is correct and it also displays the right clock-time with the helper method. Or do I understand you wrong? Probably forgot to disable clicks on the day views when a timeline is disabled. Perhaps it's only invisible. Will look into it and fix it.

maxkeppeler commented 3 years ago

@noyalj Could you explain me how to reproduce this error you have with the ClockSheet?

maxkeppeler commented 3 years ago

I close this issue for now, since I can't really figure out an issue with the ClockTimeSheet and the result time. The CalendarSheet's disabled timeline will be fixed with the next version. If you think the issue still persists, open the issue again and explain it please. Thanks!

noyize commented 3 years ago

Am still getting the wrong values even in the sample app. For example if I select 10:30 AM as time the output toast shown is not 10:30 AM. The value I am getting for 10:30 Am is 15:00:0 and for 10:30 pm is 03:00:0

maxkeppeler commented 3 years ago

The sample APK is only updated after every release, not after every commit.

Oh it's about the toast value. I will look into it.

I suspect the result time is still correct though, because implementing it in two of my apps returns the right result time in seconds. Let's see.

Thanks for letting me know.

noyize commented 3 years ago

Just checked again getting 15:00:0 for 10:30 Am.

maxkeppeler commented 3 years ago

I just inserted 10:30 am. The Toast with the result time is doing something wrong. However I have no clue where your 15:00:0 shows up? Where is that?

Screenshot_20201218_075610_com.maxkeppeler.bottomsheetssample.jpg

Screenshot_20201218_075614_com.maxkeppeler.bottomsheetssample.jpg

noyize commented 3 years ago

This is how it's showing for meScreenshot_20201218-201502.jpg Screenshot_20201218-201505.jpg

maxkeppeler commented 3 years ago

I can't replicate it. I will create a release now and then update the sample apk. Could you please check with the updated sample if this issue still persists? Perhaps I already fixed it while solving another issue.

noyize commented 3 years ago

Tested the new sample apk. It's still showing invalid time. If I select 10:30 it shows 15:00:0 and if 11:00 it shows 15:30:0 and so on.

maxkeppeler commented 3 years ago

I can't replicate this behavior. Could you just implement it and see if it works in your app? Perhaps it's because it's using the SimpleDateFormat with the current locale? What does the input in milliseconds say, is that correct?

noyize commented 3 years ago

Yes I have tried it on my own app. The result time in milliseconds are also no match. Is there anything else I can do to help.

maxkeppeler commented 3 years ago

What does it return for you in milliseconds? I just let it run, and it returns the correct time - hours are zero based though. Well, you could just clone the repo and check at which point it works wrong for you and then let me know.

maxkeppeler commented 3 years ago

Any update? Alternatively, you could also make a basic sample app with this behavior so that I can see it myself and fix it. I am still unable to see this behavior in my apps or the sample of this library.

puntogris commented 3 years ago

I'm having a similar problem, using format24Hours(true) in my case it returns 1 hour less, for example if i set the clock to 15:30 it returns 52200000 milliseconds and that would be 14:30.

I also tested this in your sample app and it returns 52200000 milliseconds too.

maxkeppeler commented 3 years ago

But that is only the case with format24Hours(true)? Thanks for verifying this issue, I will look into an fix.

puntogris commented 3 years ago

I just tried it with format24Hours(false) and it still returns 1 hour less. Thanks for the fast response.

maxkeppeler commented 3 years ago

Would it be better and easier if I directly return the hours and minutes instead of milliseconds? What do you think?

puntogris commented 3 years ago

It could be usefull, personally i mostly use clocks and timers with AlarmManager and most methods take milliseconds so it's perfect as it is for me, then i use extension functions if i ever need to convert them and display the time on screen.

maxkeppeler commented 3 years ago

I always subtracted an hour because I expected it to be a zero based hour value. But that's not what is in general expected. Thanks for informing me about it.

btraas commented 3 years ago

Not sure if it's related, but is there any reason why ClockTimeSheet shows currentTime() in UTC only? AFAIK the only way to support other timezones is to pass in a timestamp in milliseconds with a pre-calculated offset.