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

Memory leak fix, language option, tablet mode option, Context use optimize #99

Closed
wants to merge 7 commits into from

Conversation

PhilZ-cwm6
Copy link
Contributor

@PhilZ-cwm6 PhilZ-cwm6 commented May 30, 2020

See details of each commit + comments below
Utilities lib must be updated to properly use a less extensive Activity Context that can be destroyed with Activity, instead of global App Context

- do not use large App Context for short living activities
- provide proper Activity context for UI elements
Pepare for garbage collector the static methods locking the referenced context
This prevents the memory leak present on every rotation of screen in Settings or when entering/exiting any Settings tab
Using a Preferences Manager Class would further avoid using Static references to context needed by Static inner Activity classes
When editing a task starting in Land Layout, if it has a long name, title formatting is broken
When back to prortait, display is completely broken
Warning message is also moved to the scroll view
Same issue but less pronounced from Large to Portrait
…size

This is usefull for small devices when user gets used to the Settings GUI and wants a quick access to the Setings menu
@PhilZ-cwm6
Copy link
Contributor Author

PhilZ-cwm6 commented May 30, 2020

Commits details:
These are my last really big work commits as I will have less time
I will only look at code, translations and small fixes or open issues

1. Use the less global context for Activities and avoid using getApplicationContext() everywhere
Recommended for Activities (short living vs App Context, UI elements not needing a global App Context, required to properly wrap themes in future Android builds)
Using a global context doesn't always prevent memory leak anyway

2. Memory leak in ActivitySettings and all its fragments
Activity settings were leaking badly on every screen rotation and on every entering and exiting of settings menus.
It is caused by static references to the global App Context
Static context is needed by your app design for the inner class fragments, so only way is to null the static methods referencing the context

Memory Profiler: Settings menus leaking on every view or rotation
leaking

After:
no_leak

3. Fix Edit sync layout corruption
Most severe way is using a small device supporting only landscape layout, not the large layout (Nexus S in Russian system language, or lower the DP limit in isTablet() for testing in english)

  • Main view in landscape layout
  • Edit an existing SyncTask with a long name: Task name title is not well formatted
  • back to portrait: display is corrupted

In land and large layout for all languages:

  • same procedure as above but edit target to be same as master while in landscape orientation
  • warning message becomes in the scroll view
  • back to portrait: it is still in scroll view

The commit fixes the land and large layouts to match the portrait layout

Before:
Screenshot_1590430639
Screenshot_1590860611
Screenshot_1590860622
Screenshot_1590861267

After:
Screenshot_1590431100
Screenshot_1590861499

4. Option for MultiPane view on smaller devices
Really useful for small devices when user gets used to the GUI and wishes a quicker access to the settings in the landscape orientation. Else, he's stuck to the one pane view

Screenshot_1589518199

Screenshot_1589518212

Nexus S in russian
Screenshot_1589518345

5. Change app language independently from system default language
A detailed description is in the code comments

I really hope you merge this one. Suppose you have a German guy that translates your app to Deutsche. The translation could be really bad as he doesn't know about all the settings. All German people wishing a proper English translation would be stuck.
I think this opens a nice way to translate the app in more languages (but less quality translation) while keeping the freedom of a nice English translated GUI.
I also have my device in french, but I prefer using all system apps in English.

This took me a lot of work to be sure there are no bugs. I also wanted to use the new API createConfigurationContext() method with attacBaseContext() instead of the easier but deprecated updateConfiguration() method in onCreate() and onConfigurationChanged(). I also kept the support for the deprecated method.

It also revealed the small issues in code with using the global getApplicationContext() causing many GUI parts not translated. Even in Utilities lib.

During test, I fixed any compiler warning in my code and looked for memory leaks, finding the ones I fixed.

Code should be proper and bug free
Credits and sources in commit comment

Screenshot_1590870807

Screenshot_1590870839

6. A small fr translation for readability

I really hope you can merge these commits

Best regards

Sentaroh added a commit that referenced this pull request May 31, 2020
@Sentaroh
Copy link
Owner

Sentaroh commented May 31, 2020

Thank you.
Merge your change(Except Provide proper context for Activities ) and add following commit.
1.Remove the Anroid2.x compatibility code in ActivitySettings
2.added automatic restart when you change the language (same as when you change the screen theme).

P.S.
The Context for the View is derived from Activity, while the ApplicationContext() is used for all other views.

Best regards.

@Sentaroh Sentaroh force-pushed the 2.28 branch 3 times, most recently from 90a5a21 to 6ab4f0d Compare May 31, 2020 05:55
@PhilZ-cwm6
Copy link
Contributor Author

PhilZ-cwm6 commented May 31, 2020

Thank you a lot for the interest in my pull requests

There is an issue if you use App Context in Activity UI parts. You need to use the activity context to apply theme using attachBaseContext()
If you do not use the proper context, here are the non translated issues I faced and that are present in your last commit where you invoke getApplicationContext()

1.Import config Utilities lib context:
Screenshot_1590907930

2.Utilities lib create folder
Screenshot_1590909050

3.Import config msg UI context
Screenshot_1590909164

3.History msg items
Screenshot_1590908308

4.log management Utilties lib
Screenshot_1590907981

Screenshot_1590907987

5.Schedule view msgs
Screenshot_1590908315

6.Password activity msg
Screenshot_1590907951

7.Start from Shortcut notifications
Screenshot_1590910699

@PhilZ-cwm6
Copy link
Contributor Author

edited with all the translation issues related to the UI context not used

@PhilZ-cwm6
Copy link
Contributor Author

I pushed a pull request

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

Successfully merging this pull request may close these issues.

2 participants