search

grote / Transportr

Free Public Transport Assistant without Ads or Tracking
https://transportr.app
GNU General Public License v3.0
1.04k stars 187 forks source link

Fix timePicker minutes always set to current time #893

Closed Altonss closed 9 months ago

Altonss commented 10 months ago

This PR fixes issue #890.

ialokim commented 9 months ago

Thanks for the fix! Could you elaborate here (or even better add a comment) as to why this fixes the issue? Maybe a link to the timePicker Android docs?

Please also squash your commits into one for them to be merged.

Altonss commented 9 months ago

There was a strange bug, with using this code because it was only a reference to a variable that is reset when changing either hour or minute:

timePicker.currentHour = c.get(HOUR_OF_DAY)
timePicker.currentMinute = c.get(MINUTE)

So I replaced this part and cared about deprecated methods, see : https://developer.android.com/reference/android/widget/TimePicker#setCurrentHour(java.lang.Integer)

ialokim commented 9 months ago

Hum sorry I still don't get it. Is it a known issue with the deprecated setCurrent{Hour,Minute}() methods on API higher than 23, or what variable was reset when changing either hour or minute?

Altonss commented 9 months ago

Hum sorry I still don't get it. Is it a known issue with the deprecated setCurrent{Hour,Minute}() methods on API higher than 23, or what variable was reset when changing either hour or minute?

Sorry if I wasn't clear. There are 2 diffirent issues here: 1) The main issue: the c calendar variable gets reset when setting either hour or minute for timePicker. I fixed it by saving the values of the c variable before setting timePicker. 2) the deprecated API used for setCurrent which isn't really an issue, I just cleaned the code for the future.

ialokim commented 9 months ago 
1. The main issue: the c calendar variable gets reset when setting either hour or minute for timePicker. I fixed it by saving the values of the c variable before setting timePicker.

Ah, now I got it - I looked at the surrounding code and found the TimeChangedListener that updates the calendar. Nice catch!

2. the deprecated API used for setCurrent which isn't really an issue, I just cleaned the code for the future.

Thanks for that too.