Fixes #25

[kanytu]

Current approach:

• Create custom view for Champion. This view supports both ap/ad and team borders.
• Created a new tip holder for champions and changed tip builders accordingly.
• Changed other views that displayed borders with new ChampionView.
• Removed ApAdView since this is now created by ChampionView.

Sorry if this fix is a bit intrusive. Let me know your opinion. Feel free to provide code review and suggestions. Have a nice day.

[Neamar]

Looks pretty good! It's obvious you have experience in Java / Android by the way you write your code, it's much more Java-ic than mine GG!

In terms of results, it looks good, three comments:

• The gradient is nice!
• Based on the limited UX survey I did, players did not understand the AP / AD frame. I fear using the frame to indicate two different things depending on context might not help to clear this confusion, but that's a topic for another day! For now I'm happy to merge this one once we've discussed the comment above
• Finally, I don't see any point in keeping PlayerStandardTip anymore. Right now, it's only used by DoubleSmite, NoSmite and FullAPDTeam. My strategy here would be to use ChampionView everywhere and remove PlayerStandardTip completely (which will also make the remaining tips easier to understand).
• If we do suggestion 3 above, it might be worth renaming ChampionView to something more generic, such as BorderView? I guess that's up for discussion.

[Neamar]

And obviously, thank you for your help already :)

[kanytu]

So I was reading your review and here are some thoughts:

• Turning a ChampionView into a BorderView it's okay, but it will not make sense to set a champion in that border view anymore and I will have to refactor the holders to allow this change.
• Removing the PlayerStandardTip will sort of break your tips structure right now. If you remove it, the ChampionStandardTip will be used instead, and it will have to be renamed since it will not contain always champions. For that, it is necessary to create new logic in the tips adapter because, when creating a new tip, we will have to pass either an image, or a champion. I don't suggest this change. In my opinion, different tips need different holders. For different holders you need different objects that populate the views. I think the player should be renamed to image instead, creating an ImageStandardTip and ChampionStandardTip extends from this one.
• About the UX survey, that's up to you. I think we can add a walkthrough or tutorial at the start of the app explain that. If I think of a better way to display these 2 types of borders I'll let you know.

So, in order to continue I need to your know your opinion on this. I'm okay with the Tip refactor in order to make it cleaner to understand and extend. Just let me know. Otherwise, I can still perform the minor changes you asked and we think about the rest in the future.

[kanytu]

One more thing. I've noticed your server repository has been removed/set to private. Was that a strategic decision or something else? I'm asking this because I was planing to add some more tips like "Your matchup runes" and that requires some server logic. If you still want to keep it this way, where can I add suggestions and/or issues to it? (like this one, or adding the ad/ap to the counter's view)

[Neamar]

Hey there,

I like your idea of an ImageStandardTip and a ChampionStandardTip extending the first one, let's go this way :)

Once this is done, happy to merge!

Regarding your last question, I'll send you a link to the server

[Neamar]

(I've sent you access to the server. I removed it for various reasons, one of them being it's quite hard to set up correctly since it relies on Mongo, Postgres AND Redis... feel free to drop me an email at neamar@neamar.fr if you have any question)

[Neamar]

@kanytu Still interested to come back to this PR? It would be a valuable addition :)

[Neamar]

I'm gonna close for now, but if you do have time to come back to this please do! I'll be happy to merge ;)