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

Migrating to Android Support Library #493

Closed
Andy671 opened this issue Mar 2, 2017 · 19 comments
Closed

Migrating to Android Support Library #493

Andy671 opened this issue Mar 2, 2017 · 19 comments
Labels
help wanted Issues that are well-specified and don't require too much context. in progress user experience
Milestone

Comments

@Andy671
Copy link

Andy671 commented Mar 2, 2017

Software and hardware versions

Future ODK Collect releases.

Problem description

The app uses a lot of deprecated methods and classes. GSOC project aims to change UI and UX.
#464 - here you can find estimated Material Design mockups that I proposed. To start refactoring of the app we should consider to use Android Support Library that brings possibilities for the redesigning.

Other information

To make Material Theme and Support Library work we should refactor Activity to AppCompatActivity, HoloTheme to AppCompatTheme, and make a bunch of small changes, including fixes of the deprecated methods and classes. These changes should be done very carefully for maintenance of the old-device users and base functionality of the app.

@Andy671 Andy671 changed the title Migrating to Android Support Library. Migrating to Android Support Library Discussion. Mar 2, 2017
@Andy671 Andy671 changed the title Migrating to Android Support Library Discussion. [needs discussion]Migrating to Android Support Library. Mar 2, 2017
@allenagk
Copy link

allenagk commented Mar 2, 2017

Hi, I'm trying to give my support for UI/UX changes or unit tests through the GSOC project. So when we applying material themes as mentioned above we have to consider the old device users. So what is the lowest API that we should take care of? Then Do we have to configure our project for v21 and old so like that?

@shobhitagarwal1612
Copy link
Contributor

API 16

@Andy671
Copy link
Author

Andy671 commented Mar 2, 2017

Support Library will not affect minSDK level.

Starting with Support Library release 24.2.0 (August 2016), the minimum supported API level across most support libraries has increased to Android 2.3 (API level 9) for most library packages.

Reference

@lognaturel lognaturel changed the title [needs discussion]Migrating to Android Support Library. Migrating to Android Support Library Mar 2, 2017
@lognaturel
Copy link
Member

Thank you for getting the conversation started, @Andy671! What I'd like to make sure we understand in the short term is whether there are ANY UI, performance or functionality implications to changing the base classes. That will help decide if we should do it now or as part of someone's GSoC UI project. It's something I'm not too familiar so I'm interested in your experience and any references that address that question.

@Andy671
Copy link
Author

Andy671 commented Mar 2, 2017

@lognaturel We have two options here.

  1. Rewrite the app from scratch.

  2. Do research throughout the code of the app and the Support Library to inspect changes and refactor the project gradually. Android Developer Documentation is not very informative on this topic.

These two approaches are both long roads. Tomorrow, I will do a deep dive into the 2nd option, just to understand which way would be more appropriate.

@lognaturel
Copy link
Member

lognaturel commented Mar 2, 2017

Thanks @Andy671. We will do 2 because a full rewrite rarely results in something being shipped. The question is more about timing. Should this change be done now for the next release or is it better to include it with the full UI change.

@shobhitagarwal1612
Copy link
Contributor

shobhitagarwal1612 commented Mar 2, 2017

@lognaturel Maybe should add the android support library right now so that we can start replacing deprecated code being used in the app or add latest features to the app which are added to newer APIs

@Andy671
Copy link
Author

Andy671 commented Mar 3, 2017

@lognaturel Full UI change can't be done in one step because of deprecated code and inability to use new API's without refactoring huge old parts of the project. There are a lot of things should be replaced: Activity, Toolbar , TabActivity , ListActivity, etc. The heap of the Activities in the app is amiss too (modern apps use 1 activity per app). That should be replaced with Fragments. Android platform requires Fragments for Material Design patterns and modern platform API's. I propose to make one small step at a time, starting with basics. We need 3 things for the start point.

  • Activity to AppCompatActivity and Theme.Holo to Theme.AppCompat.
    Hierarchy of the AppCompatActivity:
 ↳	android.app.Activity
   	 	   ↳	android.support.v4.app.FragmentActivity
   	 	 	   ↳	android.support.v7.app.AppCompatActivity

So the behavior of the app when switching Activityto AppCompatActivity and FragmentActivityto AppCompatActivitywill stay the same.

  • But there is a method onRetainNonConfigurationInstance that became final in FragmentActivity and we can't override it now. (This method is used a lot in the app)

Retain all appropriate fragment and loader state. You can NOT override this yourself! Use onRetainCustomNonConfigurationInstance() if you want to retain your own state.

  • Alert Dialog insignificant changes.

If you allow me I can make a small pull request with good-commented commits to see what is really happening.

@mitchellsundt
Copy link
Contributor

mitchellsundt commented Mar 3, 2017 via email

@lognaturel
Copy link
Member

I agree with all of you that this needs to be done. I would like for one of you who has experience with this migration to let us know if there are any user experience implications. Are there any subtle bugs or UI changes that could be introduced or is this a completely transparent change from a user perspective.

@lognaturel
Copy link
Member

lognaturel commented Mar 3, 2017

Also, how does this interact with moving to a Fragment-based architecture? @Andy671 addresses this a little bit but perhaps we should consider making the changes in single Activities or clusters of related activities rather than making sweeping changes across the code base.

@Andy671 it sounds like you have ideas on how to answer those questions. Please do file an illustrative PR so we can discuss something concrete.

@swapsha96
Copy link

That would be great. We won't need much restructuring then. Basically, java files will be altered and not much changes in XML files would be required.

@mitchellsundt
Copy link
Contributor

mitchellsundt commented Mar 3, 2017 via email

@lognaturel
Copy link
Member

Thank you @mitchellsundt, that's very helpful!

We've made some good progress on restructuring the preference and list-based activities. As we discussed on Slack, @shobhitagarwal1612 and @dcbriccetti will have a chat about moving all the preferences activities to be fragment-based. I think that's a nice, isolated set of changes that @shobhitagarwal1612 will be able to.

Other good clusters include lists as Mitch mentioned, geo (which I think @Dvik will be filing an issue for and addressing). I think approaching the change as part of a series of refactors rather than just changing all the base classes at once will make it easier to notice any UI issues that come up and to address deeper issues. @Andy671, that may have been what you were thinking when you were offering to make an isolated sample. That would be great.

@Andy671
Copy link
Author

Andy671 commented Mar 4, 2017

@mitchellsundt, thanks for the reference.
@lognaturel, I understand, that we should do step-by-step refactor, but the problem is we can't change a few of the Activities to AppCompatActivities and leave the same other ones because of applying Theme.AppCompat . What do you mean by "isolated sample"?

@lognaturel
Copy link
Member

@Andy671 Ah, thanks for that info. If we have to apply the change to all activities at once, let's focus on addressing individual deprecated methods (#522, etc) and doing some initial restructuring of some of the activities (#507, etc) for this release cycle. We can then make the base class change at the beginning of the next one and make sure we have plenty of time to notice any potential issues.

@lognaturel
Copy link
Member

We haven't had very many across-the-board UI changes this release and probably won't in the next one so I think we might be ready to do this without too many conflicts. And if there are any UI problems, we can pinpoint them to this change.

@lognaturel lognaturel added the help wanted Issues that are well-specified and don't require too much context. label Apr 17, 2017
@shobhitagarwal1612
Copy link
Contributor

I'm working on this

@shobhitagarwal1612
Copy link
Contributor

Fixed #912

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues that are well-specified and don't require too much context. in progress user experience
Projects
None yet
Development

No branches or pull requests

6 participants