pigskin / kodi-gamepass

NFL Game Pass add-on for Kodi
Other
123 stars 83 forks source link

Show date (or weekday) of game #454

Closed AleXSR700 closed 1 year ago

AleXSR700 commented 1 year ago

Please, see https://github.com/pigskin/kodi-gamepass/issues/452

AleXSR700 commented 1 year ago

Unfortunately github shows each of my test commits which makes it look a bit weird. I have tested and seems to all work as intended :)

Feel free to pimp ;)

jm-duke commented 1 year ago

Thanks for your contribution! I'm a bit busy right now, so please give me a few days for reviewing the PR :)

Cheers, Duke

AleXSR700 commented 1 year ago

@jm-duke I made the corrections except for the dependency (as explained in my response). Of course, please feel free to change it if you have a cleaner approach. We can also adjust the layout more, but I find it quite nice as is so I did not want to mess with it more than necessary.

Of course I cannot test if all changes will work correctly when we actually have upcoming and ongoing games. But I will test that as soon as the season kicks off .

You now have write access to my temporary repo in case you want to change at source.

jm-duke commented 1 year ago

I'll test the suggested changes on my debug kodi and will merge, if everything works out.

And yes, we should re-test once the pre-season is available to see if the change messes anything up then :)

jm-duke commented 1 year ago

With the proposed changes, both settings (hide_game_length and display_datetime) were disabled and I was not able to change either. I had to change the dependency logic a bit to make it work, looks good on my end now. Can you retest on your kodi?

AleXSR700 commented 1 year ago

Thanks for re-testing. Due to the iterations my default state got changed. I will test tomorrow, yes. '!is false' and 'is true' are essentially the same in a boolean :)

AleXSR700 commented 1 year ago

Performed a re-test including deleting settings and re-installing. Everything seems to be working :)