Manually including `im.dlg:android-dialer:1.2.5` #7142

ouchadam · 2022-09-15T13:41:56Z

Type of change

Feature
Bugfix
Technical
Other :

Content

Moves all external modules to the library/external/ directory (jsonview, dialpad and diff_match_patch)
Manually includes the contents of im.dlg:android-dialer:1.2.5 as a dedicated module in library/external/dialpad. It turns out the classes we use are entirely AOSP.
Replaces AppCompat usages with stock Widgets, the build tools automatically convert these to androidx variants

Motivation and context

To allow us to avoid using the jetifier and enable Paparazzi screenshot tests within the vector module

Avoids using appcompat
avoids using an artifact without a source repository

Screenshots / GIFs

No UI changes!

Before	After

Tests

Force the dial pad to be displayed via the debug settings
Open the dial pad tab

Tested devices

Physical
Emulator
OS version(s): 28

ouchadam · 2022-09-15T13:44:40Z

library/dialpad/build.gradle

@@ -0,0 +1,20 @@
+apply plugin: 'com.android.library'


a minimal build file for the module, technically it could apply the java plugin, for consistency it uses kotlin

there's no longer any dependency on the appcompat / legacy support library

ouchadam · 2022-09-15T13:45:25Z

library/ui-styles/build.gradle

@@ -58,5 +58,5 @@ dependencies {
    // Pref theme
    implementation libs.androidx.preferenceKtx
    // dialpad dimen
-    implementation 'im.dlg:android-dialer:1.2.5'


im.dlg:android-dialer:1.2.5 is now replaced by the local module

I guess we could avoid having this dependency now, but this is not really a big deal. Same for the String resource, they could be extracted from the module dialpad module.

ouchadam · 2022-09-15T13:46:11Z

library/dialpad/src/main/java/com/android/dialer/dialpadview/DialpadTextView.java

+ * to a larger default, for the dialpad we use this class to more precisely render characters
+ * according to the precise amount of space they need.
+ */
+public class DialpadTextView extends TextView {


this has been changed from extends AppCompatTextView

ouchadam · 2022-09-15T13:46:56Z

library/dialpad/src/main/java/com/android/dialer/dialpadview/DigitsEditText.java

+import com.android.dialer.widget.ResizingTextEditText;
+
+/** EditText which suppresses IME show up. */
+public class DigitsEditText extends ResizingTextEditText {


this has been changed from extends AppCompatEditText

ouchadam · 2022-09-15T14:41:31Z

will exclude lint from this module as it's not our code

jmartinesp

It seems to work fine, so I'll trust the changes to the original library were minimal and say it LGTM 😅.

bmarty

LGTM, thanks.
When I do this type of import, I usually split into several commits for clarity.
First commit is importing the stuff, without editing anything. In this commit's comment, it's also good to link the original source location.
Next commit(s) are modification to fix issue, make the code compile, tweak the code, etc.
Doing like may also help in the future to investigate was has been changed from the original source.

bmarty · 2022-09-16T07:53:21Z

library/ui-styles/build.gradle

@@ -58,5 +58,5 @@ dependencies {
    // Pref theme
    implementation libs.androidx.preferenceKtx
    // dialpad dimen
-    implementation 'im.dlg:android-dialer:1.2.5'


I guess we could avoid having this dependency now, but this is not really a big deal. Same for the String resource, they could be extracted from the module dialpad module.

bmarty · 2022-09-16T07:57:03Z

library/dialpad/build.gradle

+
+afterEvaluate {
+    tasks.findAll { it.name.startsWith("lint") }.each {
+        it.enabled = false


lint is disabled here, because there are too many issues reported?

I wasn't sure on our stance, do we want to fix lint issues from external projects?

I can't remember the exact amount, but there were lots of lint issues about API versions, unused resources, etc. Since it's not really our code (we're just forking it to solve the mentioned issues) fixing all of those didn't seem to be worth the effort.

This is maybe not clear enough for everyone that this is external project, since it's in the library folder. Maybe create another folder like library_external, or a subfolder library/external.
But I agree, that we do not want to spend time to fix issues on all our dep.

sounds good 👍 Should I move jsonviewer as well? as I guess that technically originates from an external repository~

yes, please, and diff_match_patch too.

a9298c7 moves the modules to library/external

ouchadam · 2022-09-16T10:51:01Z

@bmarty I've updated the commits to split the change from the import f923edf

ElementBot · 2022-09-16T10:52:39Z

	Warnings
⚠️	library/external/jsonviewer/src/main/res/layout/item_jv_base_value.xml#L7 - Possible overdraw: Root element paints background `?attr/selectableItemBackground` with a theme that also paints a background (inferred theme is `@android:style/Theme.Holo`)
⚠️	library/external/jsonviewer/src/main/res/layout/item_jv_base_value.xml#L7 - Possible overdraw: Root element paints background `?attr/selectableItemBackground` with a theme that also paints a background (inferred theme is `@android:style/Theme.Holo`)

Generated by 🚫 dangerJS against 2168362

- avoids using appcompat - avoids using an artifact without a source repository

bmarty · 2022-09-19T13:55:25Z

I need this to try to try to remove jetifier from the project. Currently on develop, using this plugin id "com.dipien.byebyejetifier" version "1.2.2" and running this command: ./gradlew canISayByeByeJetifier -Pandroid.enableJetifier=false outputs:

Execution failed for task ':canISayByeByeJetifier'.
> Could not resolve all dependencies for configuration ':vector:debugAndroidTestCompileClasspath'.
   > Could not find com.android.support:support-annotations:25.3.1.
     Required by:
         project :vector > im.dlg:android-dialer:1.2.5
   > Could not find com.android.support:appcompat-v7:25.3.1.
     Required by:
         project :vector > im.dlg:android-dialer:1.2.5
   > Could not find com.android.support:design:25.3.1.
     Required by:
         project :vector > im.dlg:android-dialer:1.2.5

…ary attribute compiliation issues

ouchadam · 2022-09-20T09:09:43Z

FYI @bmarty @jmartinesp I've had to include appcompat as a dependency 2168362 as some tasks (like sonar) were attempting to compile all variants of the module but colorPrimary only existed if the parent app module was also compiled

sonarcloud · 2022-09-20T09:25:08Z

SonarCloud Quality Gate failed.

3 Bugs
0 Vulnerabilities
0 Security Hotspots
16 Code Smells

0.0% Coverage
1.9% Duplication

ouchadam requested review from a team and jmartinesp and removed request for a team September 15, 2022 13:43

ouchadam commented Sep 15, 2022

View reviewed changes

ouchadam force-pushed the feature/adm/dialpad-lib branch 2 times, most recently from 9b56ee6 to 7abcdd4 Compare September 15, 2022 15:12

ouchadam mentioned this pull request Sep 15, 2022

Paparazzi screenshot testing setup #7128

Merged

6 tasks

jmartinesp approved these changes Sep 15, 2022

View reviewed changes

bmarty approved these changes Sep 16, 2022

View reviewed changes

bmarty reviewed Sep 16, 2022

View reviewed changes

ouchadam force-pushed the feature/adm/dialpad-lib branch from 7abcdd4 to a9298c7 Compare September 16, 2022 10:50

ouchadam added 4 commits September 16, 2022 15:15

manually including the im.dlg:android-dialer:1.2.5

5058f94

- avoids using appcompat - avoids using an artifact without a source repository

disabling lint for the pulled in module

abd70d9

removing dialpad reliance on the legacy support library

e83fa9c

moving external library modules to an external directory

739a513

ouchadam force-pushed the feature/adm/dialpad-lib branch from a9298c7 to 739a513 Compare September 16, 2022 14:15

ouchadam added the Z-Merge After Release label Sep 20, 2022

including appcompat (instead of the support library) to fix colorPrim…

2168362

…ary attribute compiliation issues

ouchadam mentioned this pull request Sep 23, 2022

Bump mockk from 1.12.3 to 1.13.1 #7214

Closed

ouchadam merged commit 2c1eef7 into develop Sep 26, 2022

ouchadam deleted the feature/adm/dialpad-lib branch September 26, 2022 09:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Manually including `im.dlg:android-dialer:1.2.5` #7142

Manually including `im.dlg:android-dialer:1.2.5` #7142

ouchadam commented Sep 15, 2022 •

edited

ouchadam Sep 15, 2022

ouchadam Sep 15, 2022

ouchadam Sep 15, 2022

bmarty Sep 16, 2022

ouchadam Sep 15, 2022

ouchadam Sep 15, 2022

ouchadam commented Sep 15, 2022

jmartinesp left a comment

bmarty left a comment

bmarty Sep 16, 2022

bmarty Sep 16, 2022

ouchadam Sep 16, 2022

jmartinesp Sep 16, 2022

bmarty Sep 16, 2022

ouchadam Sep 16, 2022 •

edited

bmarty Sep 16, 2022

ouchadam Sep 16, 2022

ouchadam commented Sep 16, 2022

ElementBot commented Sep 16, 2022 •

edited

bmarty commented Sep 19, 2022

ouchadam commented Sep 20, 2022

sonarcloud bot commented Sep 20, 2022

Manually including im.dlg:android-dialer:1.2.5 #7142

Manually including im.dlg:android-dialer:1.2.5 #7142

Conversation

ouchadam commented Sep 15, 2022 • edited

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ouchadam commented Sep 15, 2022

jmartinesp left a comment

Choose a reason for hiding this comment

bmarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ouchadam Sep 16, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ouchadam commented Sep 16, 2022

ElementBot commented Sep 16, 2022 • edited

bmarty commented Sep 19, 2022

ouchadam commented Sep 20, 2022

sonarcloud bot commented Sep 20, 2022

Manually including `im.dlg:android-dialer:1.2.5` #7142

Manually including `im.dlg:android-dialer:1.2.5` #7142

ouchadam commented Sep 15, 2022 •

edited

ouchadam Sep 16, 2022 •

edited

ElementBot commented Sep 16, 2022 •

edited