butterproject / butter-android

All the cool technical bits of what used to be Popcorn Time for Android
Other
315 stars 367 forks source link

Overview strings for provider should be derived from the provider itself #285

Closed etodanik closed 6 years ago

etodanik commented 6 years ago

Right now there are hardcoded "More Movies" and "Top Movies" strings in the overview navigation. It would make more sense to derive that from the Provider, similar to getDisplayName() that exists right now on ProviderWrapper.

I'm willing to submit a PR, but first I'd like to know which approach to take, since the change will affect all provider implementations.

My suggestion is to add methods on the wrapper that would return the appropriate labels for Top/More e.t.c necessary in navigation. Any thoughts?

blazsolar commented 6 years ago

Provider API is not final yet so braking changes are not an issue.

Name should probably be combination of provider name and sorter name. So for "Top Movies" should be "defaultSorter.name provider.name".

For "More Movies" we should probably hardcore more part.

This should probably be part of TVOverviewPresenterImpl since it is only used there.

blazsolar commented 6 years ago

This was fixed and merged into develop.