search

ICT4Dat / ict4dat-news-android

ICT4D.at's App to combine all ICT4D news into one Android application
Apache License 2.0
5 stars 1 forks source link

Fix #130 - Feat remove news detail activity #138

Closed rajasone closed 5 years ago

rajasone commented 5 years ago

@spipau In last commit i move the toolbar to each fragment individually (i think it is the appropriate solution to let the Fragments handle the toolbar rather than an Activity for the sake of flexibility e.g implementing collapse toolbar is not pleasant if Activity is handling the Toolbar). I introduced one new Abstract method in BaseFragment add few lines of code to make sure fragment is handling the toolbar gracefully. It is my point of view please when you have time do the code review and if you don't like the idea of fragments containing the toolbar please just discard/ignore my last commit and we have a same old solution where Activity is handling the toolbar for this issue :)

spipau commented 5 years ago

I changed some things and think that I improved the whole thing a bit. I think your Toolbar solution is good and we should continue trying it out 👍

  1. I removed the Base Activity, since we don't need it any
  2. I improved the navigation by setting the news fragment as the starting destination. I think we should redesign the whole splash fragment
  3. Fixed some bugs and improved your navigation handling a bit
  4. Updated all the libs and implemented the breaking changes of the Alpha 8 Navigation lib.

Please review my changes and tell me what you think 😉

spipau commented 5 years ago

@rajasone can you resolve the merge conflicts? These are you changes and I don't wanna break them, thanks!

rajasone commented 5 years ago

@spipau yes sure am on it :)