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.

Sign up for GitHub

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

Jump to bottom

Make changes for when the minSdk is set to 21. #2031

Merged
merged 20 commits into from
Sep 2, 2021
Merged

Make changes for when the minSdk is set to 21. #2031

merged 20 commits into from
Sep 2, 2021

Conversation

Isira-Seneviratne
Copy link
Contributor

No description provided.

@westnordost
Copy link
Member

westnordost commented Aug 19, 2020 •
edited
Loading

Note that I will probably not review this for some time. Here is some more things that could be changed:

  • remove resources only made for versions below 21
  • replace certain Compat-functions only used for compatibility with below 21, for example Locale.toBcp47LanguageTag. Some may be marked with "LOLLIPOP" somewhere in the comment
  • replace/remove compat libraries used only for versions below 21, for example androidx.work could probably be replaced

@Isira-Seneviratne Isira-Seneviratne force-pushed the Update_minSdk_to_21 branch 2 times, most recently from b816b26 to 1678117 Compare August 27, 2020 10:28
@westnordost
Copy link
Member

Another thing that comes to my mind would be that in pre-Lollipop, referencing colors in vector drawables is not (really) possible. For many icons, there is a day and a night variant because of this. If the min-sdk is lollipop, instead the drawable can reference a color instead, so the drawables don't need to be duplicated. Example ic_building_toilets.xml

@Isira-Seneviratne
Copy link
Contributor Author

Isira-Seneviratne commented Oct 1, 2020 •
edited
Loading

Another thing that comes to my mind would be that in pre-Lollipop, referencing colors in vector drawables is not (really) possible. For many icons, there is a day and a night variant because of this. If the min-sdk is lollipop, instead the drawable can reference a color instead, so the drawables don't need to be duplicated. Example ic_building_toilets.xml

Sorry for the late reply. I'll make the change, thanks!

So what's the color value that should be used? Or should a new one be created?

@westnordost
Copy link
Member

I think a color value does not exist for it yet, as this is just one of many colors used in the vector drawables.

No issue with the late reply. After all, this PR is going to be around for a while, maybe 2 more months. And even if you abandoned this PR now, it still is valuable for the project because then later I can finish it when finally the minsdk is set to 21.

@Isira-Seneviratne Isira-Seneviratne force-pushed the Update_minSdk_to_21 branch 2 times, most recently from 5138e6a to 5dcc65e Compare October 14, 2020 02:18
@FloEdelmann FloEdelmann mentioned this pull request Jan 15, 2021
Merged
@westnordost
Copy link
Member

westnordost commented Feb 12, 2021 •
edited
Loading

Sorry, my original plan was to up the min sdk version end last year because I intended to couple that with the change to add "measurement with AR"-feature (and thus requiring camera permission). That feature wasn't developed so far after all, so that's why there was no good reason to up the min sdk version yet.

So it looks like this is going to lie around a while longer, but I will get back to it eventually. No need to bother fixing the conflicts, I will do that when the time comes.

@westnordost
Copy link
Member

Finally, one year late, I decided to finally drop support for Android 4. Thank you for your work so far on it, Isira. Quite a few things have changed in the app since you last brought this PR up to master, so there are some conflicts to solve.

I will mark this PR as "help wanted" in case other contributors to this app would like to finish up this PR (that is: solve merge conflict, check if everything still works) unless you are interested in bringing it to completion (again).

If noone else will do it, I will finish up this PR, so your work on it will merged anyway and you'll be credited for your work of course.

@westnordost westnordost added the help wanted help by contributors is appreciated; might be a good first contribution for first-timers label Jul 15, 2021
@westnordost
Merge branch 'master' into Update_minSdk_to_21
bc33d4b 
# Conflicts:
#	app/build.gradle.kts
#	app/src/main/java/de/westnordost/streetcomplete/MainActivity.java
#	app/src/main/java/de/westnordost/streetcomplete/controls/DownloadProgressFragment.kt
#	app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/changesets/OpenQuestChangesetsManager.kt
#	app/src/main/java/de/westnordost/streetcomplete/map/MainFragment.kt
#	app/src/main/java/de/westnordost/streetcomplete/map/MapFragment.kt
#	app/src/main/java/de/westnordost/streetcomplete/map/QuestPinLayerManager.kt
#	app/src/main/java/de/westnordost/streetcomplete/map/TangramPinsSpriteSheet.kt
#	app/src/main/java/de/westnordost/streetcomplete/notifications/OsmUnreadMessagesFragment.kt
#	app/src/main/java/de/westnordost/streetcomplete/quests/AbstractBottomSheetFragment.kt
#	app/src/main/java/de/westnordost/streetcomplete/quests/SplitWayFragment.kt
#	app/src/main/java/de/westnordost/streetcomplete/quests/max_speed/AddMaxSpeedForm.kt
#	app/src/main/java/de/westnordost/streetcomplete/settings/ShowQuestFormsActivity.kt
#	app/src/main/java/de/westnordost/streetcomplete/tutorial/TutorialFragment.kt
#	app/src/main/java/de/westnordost/streetcomplete/user/CircularFlagView.kt
@westnordost
ignore invalid lint warning
b488e0f
@westnordost
fix compile error
10824ce
@westnordost
fix menu item string
f7e3cfd
@westnordost
remove compat-stuff for pre lollipop
4e06bcf
@westnordost
update parent class of Application
17c7e72
@westnordost
use autoMirrored
16723b6
@westnordost
eliminate usages of resources.getDrawable
a517c7c
@westnordost
remove targetApi=21
2854cb2
@westnordost westnordost self-assigned this Aug 28, 2021
@westnordost westnordost removed the help wanted help by contributors is appreciated; might be a good first contribution for first-timers label Aug 28, 2021
@westnordost
remove resources for pre 21
e2d4f92
@westnordost
remove where possible the fillType attribute
fe69366 
because that feature was added in SDK 24, so for below that, the android build system will create raster version for those vector graphics
@westnordost
remove multiDexEnabled property
0da5d0b
@westnordost
update all those drawables and solve other warnings
1e00588
@westnordost
solve some warnings
6eda581
@westnordost
Copy link
Member

Alright, this is brought up to current master plus all kinds of optimizations done that are only available since v21. Mostly, referencing color values in vector drawables makes it possible to delete all those resources for night.

@westnordost
Copy link
Member

The resulting APK's size is 45 MB, so about 30 MB lighter than now.

@westnordost westnordost merged commit 0abc686 into streetcomplete:master Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@westnordost westnordost westnordost left review comments

Assignees

@westnordost westnordost

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Isira-Seneviratne @westnordost