Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-add option to disable favorites #231

Merged
merged 1 commit into from
May 15, 2019

Conversation

dagwieers
Copy link
Collaborator

@dagwieers dagwieers commented May 15, 2019

This PR includes:

  • Re-added the settings option to enable/disable favorites
  • Only enable My programs when settings enable it and credentials are detected
  • Reorganize unit tests (move from vrtplayer to apihelper)

This is done over concerns from #213 (comment)

Additionally, I think we need to add help to individual settings to give a little bit more insight into how settings affect the VRT NU addon. This also gives more maturity to our addon settings pages...

@dagwieers dagwieers added the enhancement New feature or request label May 15, 2019
@dagwieers dagwieers added this to the v1.10.0 milestone May 15, 2019
@dagwieers dagwieers force-pushed the favorites-optional branch 3 times, most recently from cafdc49 to fb22627 Compare May 15, 2019 15:16
@dagwieers
Copy link
Collaborator Author

@mediaminister Does this help with your concerns, provided we add help information in the Settings.

@dagwieers dagwieers force-pushed the favorites-optional branch 4 times, most recently from 66b9ed4 to 28de194 Compare May 15, 2019 20:47
This PR includes:
- Re-added the settings option to enable/disable favorites
- Only enable My programs when settings enable it and credentials are detected
- Reorganize unit tests (move from vrtplayer to apihelper)
@dagwieers dagwieers force-pushed the favorites-optional branch from 28de194 to ab6c0b7 Compare May 15, 2019 21:11
@dagwieers
Copy link
Collaborator Author

It appears that help messages don't work for addon settings, this is something for my list of Kodi shortcomings :-(

@dagwieers dagwieers merged commit 2ede6c7 into add-ons:master May 15, 2019
@mediaminister
Copy link
Collaborator

mediaminister commented May 16, 2019

Are you sure this works on a fresh Kodi installation? I can't use "my programs" on a fresh install.

My main concern is that a new function shouldn't slow down or break the basic functionality of the add-on (for new users):
These are my considerations of what the add-on should do:

  • instantly showing the main menu
  • livestreaming without login
  • asking login, downloading user data only when needed and when the user initiates this request through a listitem (or setting).
  • caching after the first user interaction, not before.

@dagwieers
Copy link
Collaborator Author

@mediaminister If you do not have any credentials, the "My programs" will not appear. As soon as you enter your credentials (and have it enabled in the settings) that menu item should become visible. This is basically what you requested. People can now use the live streams and work as before without credentials.

All your considerations are met with this implementation, and it worked fine for me. I now also tested on Windows and discovered an issue with expirationDate when using Dutch Kodi locale (on an English Windows). I have a fix for this.

@mediaminister
Copy link
Collaborator

Thanks for the explanation. It works now after providing credentials.
And many thanks for taking into account my concerns.

@dagwieers
Copy link
Collaborator Author

I appreciate your feedback and help too ! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants