opacapp / multiline-collapsingtoolbar

A modified CollapsingToolbarLayout that can deal with multiline titles
Other
782 stars 114 forks source link

Library update to 28.0.0, compile and target to 28 #61

Closed PattaFeuFeu closed 1 year ago

PattaFeuFeu commented 6 years ago

Fixes #60 by updating the support library version to 28.0.0

This also updates the compileSdk and targetSdk from version 27 (8.1) to 28 (9.0).


Another change this adds is using implementation instead of compile. As I wrote in the last commit of this request:

For quite some time now (gradle 3.0), gradle has added support for “api” and “implementation” keywords instead of “compile” which enable one to choose whether the library should propagate dependencies to its dependents (api) or merely use it for its own purposes and not make them available for modules depending on it (implementation).

For the library, this might result in breaking changes for some dependents of it as “implementation” no longer automatically adds the support libraries used by the this library to their build path, making them readily available for use in their app as well without defining them as dependencies on their own.

If you decide that this is what you’re going for as well, fine. Otherwise, feel free to remove commit 8056290 before merging.

PattaFeuFeu commented 6 years ago

Before changes

device-2018-10-16-121449

After all changes

device-2018-10-16-122024

johan12345 commented 6 years ago

Thank you very much! I tried to find out if there were any code changes between 27.1.1 and 28.0.0, but it seems that Google haven't uploaded a -sources.jar to their Maven repository for this version :/ https://dl.google.com/dl/android/maven2/com/android/support/design/28.0.0/design-28.0.0-sources.jar https://dl.google.com/dl/android/maven2/com/android/support/design/27.1.1/design-27.1.1-sources.jar

Also, maybe we should directly update to Material Components for Android 1.0.0, as seen here: https://github.com/material-components/material-components-android

PattaFeuFeu commented 6 years ago

Might make sense, yes. Should I supply a different pull request or update this one here?

johan12345 commented 6 years ago

Should I supply a different pull request or update this one here?

I think both is fine :)

Maybe you can then run a diff between the files from 27.1.1 and Material Components 1.0.0 to see if there were any changes in the 5 classes that we copied into this library (besides the renaming of packages)?

PattaFeuFeu commented 6 years ago

Sure thing! I’ll try to do that today, otherwise until the end of the week.

PattaFeuFeu commented 6 years ago

What’s your opinion regarding api VS implementation for the dependencies?

johan12345 commented 6 years ago

Hm, yeah, I think using implementation makes sense.

PattaFeuFeu commented 6 years ago

Did you select only a subset of ThemeUtils at the time? ThemeEnforcement—as it is now called—is quite a bit bigger: https://github.com/material-components/material-components-android/blob/master/lib/java/com/google/android/material/internal/ThemeEnforcement.java

johan12345 commented 6 years ago

No, that seems to be something they changed between 27.1.1 and Material Components 1.0, ThemeUtils in 27.1.1. looks basically the same as we have it currently in our library. In that case, I would copy the new version.

However, it also looks like ThemeEnforcement is now public instead of package-private. So maybe we don't need to have a copy of it in our library anymore?

PattaFeuFeu commented 6 years ago

It is public, but the @RestrictTo(LIBRARY_GROUP) leads to nasty LINT warnings, telling you that

ThemeEnforcement can only be called from within the same library group (groupId=com.android.support)

Your call. :)

johan12345 commented 6 years ago

Hm, not good either :/ Then we probably need to copy it.

PattaFeuFeu commented 6 years ago

https://github.com/opacapp/multiline-collapsingtoolbar/blob/7d39e72eb845def588054b5e9549d60ff04b5b17/multiline-collapsingtoolbar/src/main/java/net/opacapp/multilinecollapsingtoolbar/CollapsingToolbarLayout.java#L519

This line already has the same problem. ViewGroupUtils is library-private to the CoordinatorLayout library.

johan12345 commented 6 years ago

😞 If it's just a Lint warning and not an error, maybe we can live with it - we update with every support library version anyway, so its not that the library will break if they change something there.

PattaFeuFeu commented 5 years ago

For Android 9, this will become problematic—although we’ll probably see a wide-spread usage of Android Pie no earlier than 2021. 🔮

https://developer.android.com/about/versions/pie/restrictions-non-sdk-interfaces

johan12345 commented 5 years ago

Hm, but does that really affect the usage of libraries (Support Library / AndroidX) or just things that are actually in the Android SDK? I can't see any support library methods on the black- or greylists. Have you seen any crashes or warnings in the log when testing it on Android 9? I haven't seen any yet (but I also didn't explicity look for them).

Still, you have a point - the best thing would be if Google included the ability to build multiline titles into their library, so that we wouldn't have to mess around with their code and keep the library up to date ;) There's a corresponding issue in the tracker, but they marked it as "intended behavior" - on the other hand, the Material guidelines now explicitly say that long titles should be shown in multiple lines here...