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

Android: update gradle #13579

Closed
m1ga opened this issue Sep 22, 2022 · 13 comments
Closed

Android: update gradle #13579

m1ga opened this issue Sep 22, 2022 · 13 comments
Labels
android feature needs triage This issue hasn't been reviewed by maintainers

Comments

@m1ga
Copy link
Contributor

m1ga commented Sep 22, 2022

Issue: updateing to gradle > 7.1.0 breaks the SDK build.

Moving the other research parts into this ticket:

the compile errors start as soon as you go to com.android.tools.build:gradle:7.1.0 (current SDK is using 7.0.4 - https://mvnrepository.com/artifact/com.android.tools.build/gradle?repo=google)

version is set here: https://github.com/search?q=repo%3Atidev%2Ftitanium-sdk+com.android.tools.build%3Agradle%3A7.0.4&type=code (for app and modules)

more infos on gradle updates: https://github.com/tidev/titanium_mobile#upgrade-gradle-and-gradle-plugin

some other issues with gradle updates/infos:

removing

https://github.com/tidev/titanium_mobile/blob/master/android/titanium/build.gradle#L67-L69

and

https://github.com/tidev/titanium_mobile/blob/master/android/titanium/build.gradle#L76-L79

builds but of course breaks the SDK at the end. But it looks like those are the parts that will fail. It doesn't generate the files in runtime/v8/generated so #include "org.appcelerator.kroll.KrollModule.h" will fail during compiling. My guess is that the order of the gradle tasks changed and c++ are not generated before the cmake part

The gradle command ./gradlew :titanium:externalNativeBuildCleanDebug will already trigger the error.

There is a google issue https://issuetracker.google.com/issues/232060576 AGP 7.1 and higher does not build :nativelib module incrementally with "As a workaround we are downgrading AGP." which looks like our issue too. I can remove the .cxx folder and the clean part works but then it breaks at :titanium:buildCMakeRelWithDebInfo[arm64-v8a] with fatal error: 'org.appcelerator.kroll.KrollModule.h' file not found since the files aren't there.

Material lib version

The current version is 1.6.1. If we want to use >=1.7.0 (https://github.com/material-components/material-components-android/releases?expanded=true&page=2&q=1.7.0) we have to use a higher gradle-plugin version

@m1ga m1ga added bug needs triage This issue hasn't been reviewed by maintainers labels Sep 22, 2022
@m1ga
Copy link
Contributor Author

m1ga commented Sep 22, 2022

Interesting part:
if you use Studio to run the gradle task :titanium:assembleRelease it will still fail with the same error but continues with other task. After that you can run npm run build (without the clean) and the files are there.

so there is some kind of order error

@hansemannn
Copy link
Collaborator

cc @janvennemann

@AbdullahFaqeir
Copy link
Contributor

CC @hansemannn @janvennemann @m1ga

I would love to share this file with you guys after almost 40 hours of working and this is my third day awake.

titanium-sdk-gradle-8.3.1-build.log

I've upgraded Gradle version to the very latest 8.3.1 and I've completely fixed the tasks execution order along side some small fixes to deprecated apis.

@m1ga
Copy link
Contributor Author

m1ga commented Mar 21, 2024

😮 sounds great! Besides the 3rd day awake part 😄 Looking forward to test it!!!

@m1ga
Copy link
Contributor Author

m1ga commented Mar 22, 2024

To make the real changes a bit easier to check later:

can we merge this one first: #13888 ?
It is only applying Android Studio recommendations about the style:

    Consider using 'withType(...).configureEach' to avoid unnecessary configuration
    Consider using 'tasks.register' to avoid unnecessary configuration

no code changes in this PR but as this is part of the big gradle change coming up it will make it easier to test in steps.

And since there will be a CMake update too: #13966
Don't worry: looks big, but all Java files are just linting because I've raised checkstyle and it's a bit stricter now 😄

@m1ga
Copy link
Contributor Author

m1ga commented Mar 26, 2024

Huge congrats to @AbdullahFaqeir 🎉

The #14014 works! I was able to build kitchensink with it and my app that uses a few modules. I still have to do some more tests but the SDK builds 👍

Still would like to merge in stages so it's less to review and test. These would be my proposed order:

  1. chore: update gradle files #13888 - styling recommendations
  2. chore(android): update cmake, checkstyle #13966 - cmake + checkstyle update
  3. chore(android): gradle 8 #14014 - his gradle stuff (will be a lot less changes when 1 is merged)
  4. if we want: chore(android): prepare SDK for Android 34 #13940 - so we can even use more modules (requires more testing, not required by the Playstore at the moment)

@trkfabi
Copy link

trkfabi commented Jun 25, 2024

The #14014 works for me! I was able to update a custom Android Stripe module using the latest Stripe SDK (which needs Material 1.11.0) and compile my app with the new Material theme, a new Gradle version and the compile target set to 34.
Minor issues with some UI components which I will try to workaround.

It would be great if this PR gets merged !

@hansemannn
Copy link
Collaborator

@trkfabi Thank you! If you can report the style issues, we can look into these as well

@trkfabi
Copy link

trkfabi commented Jun 25, 2024

@hansemannn I just noticed that all the textfields' background are black now but I think this happens because I use a custom theme for this app which has not been updated in a while.

I found a workaround by adding a few lines to /platform/android/res/values/mytheme.xml:

<style name="Theme.NoActionBar" parent="@style/Theme.MaterialComponents">
        .... 
        ....

	<!-- Add custom text field styles -->
        <item name="textInputStyle">@style/CustomTextInputLayout</item>
</style>

  <!-- Custom style for TextInputLayout -->
  <style name="CustomTextInputLayout" parent="Widget.MaterialComponents.TextInputLayout.FilledBox">
      <item name="boxBackgroundColor">#FFFFFF</item>
      <item name="hintTextColor">?attr/colorOnSurface</item>
      <item name="boxStrokeColor">?attr/colorPrimary</item>
  </style>
  <!-- Custom style for TextInputEditText -->
  <style name="CustomTextInputEditText" parent="Widget.MaterialComponents.TextInputEditText.FilledBox">
      <item name="android:background">#FFFFFF</item>
      <item name="android:textColor">?android:attr/textColorPrimary</item>
  </style>

So I guess this may not be an issue if you use a default Material theme.

@trkfabi
Copy link

trkfabi commented Jun 27, 2024

@m1ga @hansemannn Everything looks good on the app except some <TextField> styling.
The background was black as a I mentioned before and backgroundColor was not having effect, so I set a custom style with white background, but I realised that I need it to be gray in some places. This property is not working.
I had to add some custom style to change the boxStrokeColor property too to remove the black underline in the TextField and also change it to blue when focused. Is there a way to do this by code instead of messing with the theme?

<item name="boxStrokeColor">@color/textinput_stroke_color</item> and the text input_stroke_color file is:

<?xml version="1.0" encoding="utf-8"?>
<selector xmlns:android="http://schemas.android.com/apk/res/android">
    <item android:color="#02B3E4" android:state_focused="true"/>
    <item android:color="#00FFFFFF" android:state_hovered="true"/>
    <item android:color="#00FFFFFF" android:state_enabled="false"/>
    <item android:color="#00FFFFFF"/>
</selector>

@m1ga
Copy link
Contributor Author

m1ga commented Jun 27, 2024

I would need to look into that. There are some default styles for the Titanium themes.

As mentioned in the older post I would start with the first two PRs (#13579 (comment)) before updating gradle. Those only update gradle syntax and cmake but still runs the old gradle and shouldn't break anything.

After those are merged the gradle PR is smaller and easier to see those style changes and how to fix those.

@trkfabi
Copy link

trkfabi commented Jun 27, 2024

I was wondering the same as in line #190 hehe:

if (d.containsKey(TiC.PROPERTY_BACKGROUND_COLOR)) {

If I change line #191 to tv.setBackgroundColor(TiConvert.toColor(d, TiC.PROPERTY_BACKGROUND_COLOR, proxy.getActivity())); it seems to work fine

@m1ga
Copy link
Contributor Author

m1ga commented Jun 27, 2024

It looks like that was used to remove the underline: #9361 (review)
and search for Will this remove the underline even if no custom background was applied?

@m1ga m1ga closed this as completed Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android feature needs triage This issue hasn't been reviewed by maintainers
Projects
None yet
Development

No branches or pull requests

4 participants