ListView Android performance refactor

SusanRatiLane commented 2 months ago

General items:

[x] ant tests passes on my machine
[X] I branched from ucr
[X] My pull request has ucr as the base

Further, if you've changed the blocks language or another user-facing designer/blocks API (added a SimpleProperty, etc.):

[ ] I have updated the corresponding version number in YaVersion.java
[ ] I have updated the corresponding upgrader in YoungAndroidFormUpgrader.java (components only)
[ ] I have updated the corresponding entries in versioning.js

^^ I have actually never done this, so we'll need to talk.

Description

This is a performance refactor submitted by @patryk84a in February 2024. He also included some methods for adding and removing items that the class should probably have. I included implementations for these in iOS.

He also included one aesthetic property for which I included a stub for iOS.

Fixes #2857

Original PR (now closed): #3099

Co-author: @patryk84a

SusanRatiLane commented 2 months ago

Hmm. I thought the tests were passing, but two ListView ones are not. Patryk's changes have broken an assumption the tests are making, but I'm trying to sort out what it is.

ETA: Tests now passing.

patryk84a commented 2 months ago

The HintSearchText property has been removed. Maybe instead for Android add a fixed hint text but from the Android resources "@android:string/search_go", then we should get the string in the language set in the phone, without the possibility of defining your own, which would be compatible with IOS

SusanRatiLane commented 2 months ago

The HintSearchText property has been removed. Maybe instead for Android add a fixed hint text but from the Android resources "@android:string/search_go", then we should get the string in the language set in the phone, without the possibility of defining your own, which would be compatible with IOS

Actually, that was removed because someone else wrote a PR that adds the property for both Android and iOS. I should probably have mentioned that in the PR description.

patryk84a commented 2 months ago

I guess I can't make any changes here, right?

SusanRatiLane commented 2 months ago

I guess I can't make any changes here, right?

I should be able to give you access. I'll take a look.

patryk84a commented 1 month ago

I think we need to hide the Multiselect function, which is not finished. But with a few additional blocks we can use it, we just need to build and control the list of selected elements.

patryk84a commented 3 weeks ago

Fix duplicate call to notifyDataSetChanged - I don't really understand this fix because I didn't find a duplicate call to the notifyDataSetChanged() method. How is it supposed to help to place this method in several places instead of in the clearSelection() function, since this function is called anyway.

Any syntax errors, whitespace, etc. were caused by working in an editor without intelij. I think my current code editor would catch those errors now. It's great that there are public methods related to Multiselect. We'll be able to use it in the extension.

I tried to reflect the appearance of the new list view in the Mock file in gwt, but I can't make these components behave the way I want them to. The problem is probably with automatic dimensioning, e.g. to the width of the parent, etc. In gwt we probably have to provide everything in constant calculated dimensions.

Is it possible to update the test server?

patryk84a commented 3 weeks ago

Ok, I tested it and it seems to work fine. There is one small detail that I didn't notice before. There is no margin between the image and the text. The text is glued directly to the image. I think a quick fix is to add a margin to the right of the image, then it will work for every layout.

layoutParamsImage.setMargins(0, 0, 15, 0);

The padding for the element is set to 15, so I think the margin can also be set to 15 so there will be equal space around the image.

Edit. I have a problem compiling a simple project. It looks like a problem with dynamicanimation. It's a simple empty project, with a ListView added. Oddly enough, my other project with the extension I'm testing builds correctly.

App Inventor is unable to compile this project.
The compiler error output was
[ReadBuildInfo] Starting Task
[ReadBuildInfo] Task succeeded in 0.005 seconds
[LoadComponentInfo] Starting Task
[LoadComponentInfo] INFO: Generating assets...
[LoadComponentInfo] Component assets needed, n = 0
[LoadComponentInfo] INFO: Generating activities...
[LoadComponentInfo] Component activities needed, n = 0
[LoadComponentInfo] Component activity metadata needed, n = 0
[LoadComponentInfo] INFO: Generating broadcast receivers...
[LoadComponentInfo] Component content providers needed, n = 0
[LoadComponentInfo] INFO: Generating libraries...
[LoadComponentInfo] Libraries needed, n = 4
[LoadComponentInfo] Component metadata needed, n = 0
[LoadComponentInfo] INFO: Generating Android minimum SDK...
[LoadComponentInfo] INFO: Generating native libraries...
[LoadComponentInfo] Native Libraries needed, n = 0
[LoadComponentInfo] INFO: Generating permissions...
[LoadComponentInfo] usesLocation = False
[LoadComponentInfo] Permissions needed, n = 3
[LoadComponentInfo] Component services needed, n = 0
[LoadComponentInfo] INFO: Generating component broadcast receivers...
[LoadComponentInfo] Task succeeded in 0.005 seconds
[PrepareAppIcon] Starting Task
[PrepareAppIcon] INFO: Creating mipmap dirs...
[PrepareAppIcon] INFO: Generating icons...
[PrepareAppIcon] Generating icons for mipmap-mdpi
[PrepareAppIcon] Generating icons for mipmap-hdpi
[PrepareAppIcon] Generating icons for mipmap-xhdpi
[PrepareAppIcon] Generating icons for mipmap-xxhdpi
[PrepareAppIcon] Generating icons for mipmap-xxxhdpi
[PrepareAppIcon] Task succeeded in 1.44 seconds
[XmlConfig] Starting Task
[XmlConfig] INFO: Creating animation xml
[XmlConfig] Creating zoom_enter.xml
[XmlConfig] Creating fadeout.xml
[XmlConfig] Creating slide_v_exit.xml
[XmlConfig] Creating fadein.xml
[XmlConfig] Creating zoom_exit.xml
[XmlConfig] Creating slide_v_enter.xml
[XmlConfig] Creating zoom_exit_reverse.xml
[XmlConfig] Creating slide_v_enter_reverse.xml
[XmlConfig] Creating zoom_enter_reverse.xml
[XmlConfig] Creating slide_enter_reverse.xml
[XmlConfig] Creating slide_exit.xml
[XmlConfig] Creating hold.xml
[XmlConfig] Creating slide_enter.xml
[XmlConfig] Creating slide_v_exit_reverse.xml
[XmlConfig] Creating slide_exit_reverse.xml
[XmlConfig] INFO: Creating style xml
[XmlConfig] INFO: Creating provider_path xml
[XmlConfig] INFO: Creating network_security_config xml
[XmlConfig] INFO: Generating adaptive icon file
[XmlConfig] INFO: Generating round adaptive icon file
[XmlConfig] INFO: Generating adaptive icon background file
[XmlConfig] Task succeeded in 0.023 seconds
[CreateManifest] Starting Task
[CreateManifest] INFO: Reading project specs...
[CreateManifest] VCode: 1
[CreateManifest] VName: 1.0
[CreateManifest] Min SDK 7
[CreateManifest] INFO: Writing screen 'appinventor.ai_froniu84a.TestList2.Screen1'
[CreateManifest] Task succeeded in 0.004 seconds
[AttachNativeLibs] Starting Task
[AttachNativeLibs] Task succeeded in 0.001 seconds
[AttachAarLibs] Starting Task
[AttachAarLibs] Task succeeded in 0.255 seconds
[AttachCompAssets] Starting Task
[AttachCompAssets] Task succeeded in 0.006 seconds
[MergeResources] Starting Task
[MergeResources] Task succeeded in 0.39 seconds
[SetupLibs] Starting Task
[SetupLibs] Task succeeded in 0.0 seconds
[RunAapt] Starting Task
[RunAapt] Task succeeded in 1.488 seconds
[GenerateClasses] Starting Task
[GenerateClasses] INFO: Source File: appinventor/ai_froniu84a/TestList2/Screen1.yail
[GenerateClasses] INFO: Libraries Classpath = /tmp/kawa14467220630049916766.jar:/tmp/acra-4.4.04386712429333399392.jar:/tmp/AndroidRuntime16120822487433513442.jar:/tmp/annotation13014115219260585560.jar:/tmp/annotation-experimental12348530837164364198.jar:/tmp/appcompat3618053516342495784.jar:/tmp/asynclayoutinflater11036814901298295053.jar:/tmp/collection15386727478201896632.jar:/tmp/constraintlayout8435834626733006340.jar:/tmp/constraintlayout-solver16881849647986783517.jar:/tmp/coordinatorlayout17375063022588164935.jar:/tmp/core6678990835176004836.jar:/tmp/core-common11842458198059626815.jar:/tmp/core-runtime14980001937012704416.jar:/tmp/cursoradapter12777841220586366220.jar:/tmp/customview18261565148565848005.jar:/tmp/documentfile6220625705361748242.jar:/tmp/drawerlayout11471739395439869705.jar:/tmp/fragment95684354356111811.jar:/tmp/interpolator11539032866908444388.jar:/tmp/legacy-support-core-ui13526428991513421884.jar:/tmp/legacy-support-core-utils11765526683521597153.jar:/tmp/lifecycle-common12922577050493980387.jar:/tmp/lifecycle-livedata13303049491677204274.jar:/tmp/lifecycle-livedata-core12495346542997443995.jar:/tmp/lifecycle-runtime16205726281780249935.jar:/tmp/lifecycle-viewmodel17754438915619659893.jar:/tmp/loader14853515651910499771.jar:/tmp/localbroadcastmanager8300390504258102368.jar:/tmp/print17318706140083364454.jar:/tmp/slidingpanelayout11255491935993464107.jar:/tmp/swiperefreshlayout1510348642052479657.jar:/tmp/vectordrawable14717579087891322720.jar:/tmp/vectordrawable-animated18341933605144306689.jar:/tmp/versionedparcelable3036412154943123427.jar:/tmp/viewpager11701451848359202873.jar:/tmp/recyclerview14765655933166983797.jar:/tmp/cardview12308154855056277619.jar:/tmp/dynamicanimation16113427183930132432.jar:/tmp/1730427253857_2036230946920954880-0/youngandroidproject/../build/classes:/tmp/android3746564125191541419.jar
(compiling appinventor/ai_froniu84a/TestList2/Screen1.yail to appinventor.ai_froniu84a.TestList2.Screen1)
(compiling /tmp/runtime5735272458664150400.scm to com.google.youngandroid.runtime)
[GenerateClasses] Task succeeded in 4.198 seconds
[RunD8] Starting Task
[RunD8] INFO: Using pre-dexed dex-cached-d9af37ea01412c4d7a686a9f8b9e5cc9.dex <- /tmp/AndroidRuntime16120822487433513442.jar
[RunD8] INFO: Using pre-dexed dex-cached-4afd1465d334dee94ac700edb0374a45.dex <- /tmp/kawa14467220630049916766.jar
[RunD8] INFO: Using pre-dexed dex-cached-558104d32e109ad96655ecbe9fe4e39f.dex <- /tmp/annotation13014115219260585560.jar
[RunD8] INFO: Using pre-dexed dex-cached-ef239d31d31d166f7410d71e14db1a68.dex <- /tmp/swiperefreshlayout1510348642052479657.jar
[RunD8] INFO: Using pre-dexed dex-cached-de87276390f7a8c71064f3e7908c143b.dex <- /tmp/core-common11842458198059626815.jar
[RunD8] INFO: Using pre-dexed dex-cached-c950a9845cca4523f19bc2fc922b41bf.dex <- /tmp/legacy-support-core-utils11765526683521597153.jar
[RunD8] INFO: Using pre-dexed dex-cached-35076b6f638faca00c217e98a0385344.dex <- /tmp/interpolator11539032866908444388.jar
[RunD8] INFO: Using pre-dexed dex-cached-bee920fd1e059913bf2d820dbb234720.dex <- /tmp/lifecycle-common12922577050493980387.jar
[RunD8] INFO: Using pre-dexed dex-cached-1e4d689047e2ee2cc76b7203626e9c33.dex <- /tmp/customview18261565148565848005.jar
[RunD8] INFO: Using pre-dexed dex-cached-74360288e1016af9d6afdd39a2caa1b8.dex <- /tmp/asynclayoutinflater11036814901298295053.jar
[RunD8] INFO: Using pre-dexed dex-cached-d53c4c8eedbf41663110b03a33fe9914.dex <- /tmp/lifecycle-runtime16205726281780249935.jar
[RunD8] INFO: Using pre-dexed dex-cached-032c68a2c5e93c1baac1524e105bbe88.dex <- /tmp/collection15386727478201896632.jar
[RunD8] INFO: Using pre-dexed dex-cached-91b7d1f3cb5fd0fd2f3a4c939316139b.dex <- /tmp/versionedparcelable3036412154943123427.jar
[RunD8] INFO: Using pre-dexed dex-cached-16ea1fc3b7e49139559eafccec2708ab.dex <- /tmp/lifecycle-viewmodel17754438915619659893.jar
[RunD8] INFO: Using pre-dexed dex-cached-84637f838557e65ad5856467fd22316c.dex <- /tmp/print17318706140083364454.jar
[RunD8] INFO: Using pre-dexed dex-cached-a9e803bf51318b35fcaf603f59f25f7c.dex <- /tmp/vectordrawable14717579087891322720.jar
[RunD8] INFO: Using pre-dexed dex-cached-b73957ec00af79319f56aedf27dff3fc.dex <- /tmp/core-runtime14980001937012704416.jar
[RunD8] INFO: Using pre-dexed dex-cached-4e788f5c66d33a47036ae77a849ffa1b.dex <- /tmp/core6678990835176004836.jar
[RunD8] INFO: Using pre-dexed dex-cached-e890ce7827a13b8714afbb241f7e89b2.dex <- /tmp/lifecycle-livedata-core12495346542997443995.jar
[RunD8] INFO: Using pre-dexed dex-cached-9e7da29c234333f4d245d8baa48e1746.dex <- /tmp/annotation-experimental12348530837164364198.jar
[RunD8] INFO: Using pre-dexed dex-cached-81757b9148a3984b5874ef1fd300acdb.dex <- /tmp/documentfile6220625705361748242.jar
[RunD8] INFO: Using pre-dexed dex-cached-b83403f2b639cc6e7a6f9c0681d69367.dex <- /tmp/viewpager11701451848359202873.jar
[RunD8] INFO: Using pre-dexed dex-cached-c2a16bb02f51a6fcd1ff46dd1a78d0bf.dex <- /tmp/slidingpanelayout11255491935993464107.jar
[RunD8] INFO: Using pre-dexed dex-cached-2eabb48fdf784357d984acbda18ea6a3.dex <- /tmp/appcompat3618053516342495784.jar
[RunD8] INFO: Using pre-dexed dex-cached-7a5a85c38b934be86a121ca9b5e7477b.dex <- /tmp/vectordrawable-animated18341933605144306689.jar
[RunD8] INFO: Using pre-dexed dex-cached-a3a712d1d179d231b40a977ed5593202.dex <- /tmp/lifecycle-livedata13303049491677204274.jar
[RunD8] INFO: Using pre-dexed dex-cached-9b1768a7734de4eaf5a635a007ad4b25.dex <- /tmp/coordinatorlayout17375063022588164935.jar
[RunD8] INFO: Using pre-dexed dex-cached-d3d338969c3cd4ad53a888753da3b580.dex <- /tmp/cursoradapter12777841220586366220.jar
[RunD8] INFO: Using pre-dexed dex-cached-a116e0a41475b6522ae295f7cc4ff4b3.dex <- /tmp/legacy-support-core-ui13526428991513421884.jar
[RunD8] INFO: Using pre-dexed dex-cached-a07a23b88daaae0b48cdd25378e03b03.dex <- /tmp/localbroadcastmanager8300390504258102368.jar
[RunD8] INFO: Using pre-dexed dex-cached-bb8901219e7be6c576cc65e75503d7fa.dex <- /tmp/fragment95684354356111811.jar
[RunD8] INFO: Using pre-dexed dex-cached-fca718e99e97cb52f4c8c0ada2c74e72.dex <- /tmp/drawerlayout11471739395439869705.jar
[RunD8] INFO: Using pre-dexed dex-cached-ccb6f0c5e0adc11059c3d609a2fd15d6.dex <- /tmp/loader14853515651910499771.jar
[RunD8] INFO: Using pre-dexed dex-cached-c0e4dda29672c028ea3ce0e3785040de.dex <- /tmp/constraintlayout8435834626733006340.jar
[RunD8] INFO: Using pre-dexed dex-cached-85f46a2fd295a23f59a391cccd7a11c8.dex <- /tmp/constraintlayout-solver16881849647986783517.jar
[RunD8] INFO: Using pre-dexed dex-cached-1bc51353c62ddc7fef531c4bbfd9e480.dex <- /tmp/cardview12308154855056277619.jar
[RunD8] INFO: Using pre-dexed dex-cached-742155d268fadbad92adb35be882bd67.dex <- /tmp/recyclerview14765655933166983797.jar
[RunD8] INFO: Using pre-dexed dex-cached-ae0a0504ff198c23ba89f60cba6bee1b.dex <- /tmp/dynamicanimation16113427183930132432.jar
[RunD8] ERROR: d8 failed.
[RunD8] Task errored in 1.771 seconds

jisqyv commented 2 weeks ago

I just pushed up the squashed version, after merging in ucr

jisqyv commented 2 weeks ago

Additional info. The filter bar is there, but it has a black background, while with the companion, the background was white.

jisqyv commented 2 weeks ago

I'm removing my objections. The behavior I'm seeing (black search bar) is also present in our current production system, so this is not a new bug.

ewpatton commented 2 weeks ago

Note that I had previously fixed those comments in the individual changes prior the squash.

patryk84a commented 2 weeks ago

Additional info. The filter bar is there, but it has a black background, while with the companion, the background was white.

Yes, I've noticed that too. The theme type auto-detection doesn't work very well. I think it used to work when phones didn't have a built-in dark theme. One idea was to set the EditText background to transparent so that it takes the ListView background, and set the search text color to the MainText color. If the app author sets the ListView colors to the appropriate ones, the search bar will also have the appropriate colors. We don't have to decide the colors for the app author.

mit-cml / appinventor-sources

ListView Android performance refactor #3235