fix(android): fragments in children of header/footerview #12524

hansemannn · 2021-03-07T21:32:39Z

All credits to @drauggres!

JIRA:
https://jira.appcelerator.org/browse/TIMOB-28376

MapView Test:

Build and run the code in TIMOB-28376's "description" on Android.
Verify it does not crash and shows a map in the ListView.

ListView Dark/Light Switch Test:

Build and run ListViewHeaderViewTest.js attached to TIMOB-28376 on Android 10 or higher.
Once the app starts, tap on the "Home" button. (Do not back out.)
Change Android's Dark theme under "Settings/Display".
Resume the Titanium app.
Verify header/footer text color has changed to the same color as the row text.

TableView Dark/Light Switch Test:

Build and run TableViewHeaderLabelTest.js attached to TIMOB-28376 on Android 10 or higher.
Once the app starts, tap on the "Home" button. (Do not back out.)
Change Android's Dark theme under "Settings/Display".
Resume the Titanium app.
Verify header/footer text color has changed to the same color as the row text.

specifically allow to add a MapView ("ti.map") inside headerView of Ti.UI.ListView fix TIMOB-28376

build · 2021-03-07T21:50:26Z

	Fails
🚫	Tests have failed, see below for more information.
🚫	🔬 There are library changes, but no changes to the unit tests. That's OK as long as you're refactoring existing code, but will require an admin to merge this PR. Please see README.md#unit-tests for docs on unit testing.

	Messages
📖	✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖	❌ 1 tests have failed There are 1 tests failing and 960 skipped out of 16022 total tests.
📖	🎉 Another contribution from our awesome community member, hansemannn! Thanks again for helping us make Titanium SDK better. 👍
📖	💾 Here's the generated SDK zipfile.

Tests:

Classname	Name	Time	Error
ios.macos.Titanium.Blob	image dimensions should be reported in pixels (10.15.5)	0.049	Error: expected 6 to be 11 value@file:///node_modules/should/cjs/should.js:356:23 postlayout@file:///ti.blob.test.js:488:33

Generated by 🚫 dangerJS against a2e7d88

m1ga · 2021-03-08T11:32:25Z

👍 for the community!

should be the same here I guess: https://github.com/appcelerator/titanium_mobile/blob/b6436261f78d53f37c6ddcb461c612345f96e493/android/modules/ui/src/java/ti/modules/titanium/ui/widget/tableview/TableViewHolder.java#L538
at least the crash is the same when you exchange listview <-> tableview

…mobile into TIMOB-28376

hansemannn · 2021-03-08T12:01:46Z

@m1ga Updated!

…e changes

garymathews

LGTM!

ssjsamir

FR Passed, tested using the test cases above.

Test Environment

MacOS Big Sur: 11.1 Beta 1
Xcode: 12.2 Beta
Java Version: 1.8.0_242
Android NDK: 21.3.6528147
Node.js: 12.18.1
""NPM":"5.0.0","CLI":"8.1.1""
iPhone 8 14.2

build · 2021-03-11T14:45:53Z

The backport to 10_0_X failed:

The process 'git' failed with exit code 128

Check the run for full details
To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Check out the target branch
git checkout 10_0_X
# Make sure it's up to date
git pull
# Check out your branch
git checkout -b backport-12524-to-10_0_X
# Apply the commits from the PR
curl -s https://github.com/appcelerator/titanium_mobile/commit/896e25485fdda00f4f396b16bb5486a74681e52e.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/ecc0d57281161c18054f4ad0a0c8de64530255fa.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/ed3cf84a3261775954faceda40af68a1bb97aa7e.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/936eb59192e195fba995603d09d552fa7c90392c.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/1eceea723c1ffc64997e7b13bb6885347d652ee8.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/bc3e78ac58339c1f729c78a0f5ee84179a62699b.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/a2e7d885df1d136c06d082669d9274aab36dc90d.patch | git am -3 --ignore-whitespace
# Push it to GitHub
git push --set-upstream origin backport-12524-to-10_0_X

Then, create a pull request where the base branch is 10_0_X and the compare/head branch is backport-12524-to-10_0_X.

…/footerview (#12524) specifically allow to add a MapView ("ti.map") inside headerView of Ti.UI.ListView Fixes TIMOB-28376

hansemannn · 2022-01-13T14:47:59Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/listview/ListSectionProxy.java

+		}
+		if (hasPropertyAndNotNull(TiC.PROPERTY_FOOTER_VIEW)) {
+			final TiViewProxy footerProxy = (TiViewProxy) getProperty(TiC.PROPERTY_FOOTER_VIEW);
+			footerProxy.releaseViews();


@drauggres Quick question as you fixed this one and https://jira.appcelerator.org/browse/TIMOB-28583 now popped up for us: When comparing the changes, you release the views and set the activity for list headers / footers, but you don't set the activity for list section headers / footers. Is this maybe the missing piece to prevent further crashes?

Hi. The original commit I made is here:
NetrisTV@f9c1912

IIRC the problem here is that the proxies in Titanium receive an Activity at the creation time from the currently open Window, which may not be the Window where they will be added to later.
We should set the activity for any child views to the same as the parent.

I'm not sure about .releaseViews() part, I didn't write it. This is probably for the case where you are moving a view from one parent to another. I don't know if this is really necessary.

Okay, thank you! So you would recommedn to add the setActivity here as well? And I am wondering if a workaround then may be to wait for the window to be opened!

but you don't set the activity for list section headers / footers

Just checked the code, we call setActivity in this case too:
https://github.com/appcelerator/titanium_mobile/blob/2dad43e47e6db455ce6d9f852082ab766bfd81d8/android/modules/ui/src/java/ti/modules/titanium/ui/widget/listview/ListSectionProxy.java#L350-L365

In theory it is possible to have a "wrong" Activity stored in the parent proxy. We don't really know the right one before it is being added to a Window.
I think we should override ListViewProxy.add to call setActivity for its header/footer and every section (and their headers/footers).
https://github.com/appcelerator/titanium_mobile/blob/2dad43e47e6db455ce6d9f852082ab766bfd81d8/android/titanium/src/java/org/appcelerator/titanium/proxy/TiViewProxy.java#L581

UPD: And this is probably not correct, we should always update activity:
https://github.com/appcelerator/titanium_mobile/blob/2dad43e47e6db455ce6d9f852082ab766bfd81d8/android/titanium/src/java/org/appcelerator/titanium/proxy/TiViewProxy.java#L616-L617

And I am wondering if a workaround then may be to wait for the window to be opened!

just tried that and it doesn't work.

Btw: it just happens with a map, a normal view inside the footerView works fine. And if your footerView is visible directly it works with a map too.

It starts some map parts

I/Google Maps Android API: Google Play services package version: 214815044 I/Google Maps Android API: Google Play services maps renderer version(legacy): 203115000

before it crashs. So perhaps we have to search in ti.map?

Also it will crash when the map is inside a normal template item/row.

I can get it to work when I call mapView.add(require('ti.map').createView()); when the footer view is visible.

So as a workaround @hansemannn you could add #13095 and use the scrolling event to get the firstVisibleItemIndex all the time and check if that is near the bottom and add the mapview to the view.
Not sure if there is an event of the footerView so you'll know that it is created by the recyclerview

That's really hard to do in the quite extended environment we're using it here. It really has to be a ti.map thing then. Not sure where to look then.

@hansemannn there is an easier workaround:

const mapView = Ti.UI.createView({ height: 200 }); mapView.addEventListener("postlayout", postLay); function postLay() { mapView.add(require('ti.map').createView()); mapView.removeEventListener("postlayout", postLay) } // footerView: mapView

add a "loading..." text into the mapView and you're fine 😉 Works in my test app.

Also found some older issues with map view fragments in other fragments:
https://jira.appcelerator.org/browse/TIMOB-18244
https://jira.appcelerator.org/browse/TIMOB-18876

Tested fragmentOnly: true but it will only show an empty view, no map.

Interesting - will test that on Monday as the first thing. Thank you!

fix(android): fragments in children of header-/footerview

896e254

specifically allow to add a MapView ("ti.map") inside headerView of Ti.UI.ListView fix TIMOB-28376

build added this to the 10.1.0 milestone Mar 7, 2021

build requested a review from a team March 7, 2021 21:50

build added android community 🔥 needs tests 🚨 labels Mar 7, 2021

Merge branch 'master' into TIMOB-28376

b643626

hansemannn added 2 commits March 8, 2021 13:00

chore(android): handle table-view header/footer as well

ecc0d57

Merge branch 'TIMOB-28376' of https://github.com/hansemannn/titanium_…

ed3cf84

…mobile into TIMOB-28376

hansemannn and others added 2 commits March 8, 2021 13:10

fix: fix typo

936eb59

fix(android): minor cleanup

1eceea7

garymathews approved these changes Mar 8, 2021

View reviewed changes

build added the in-qe-testing 🕵 label Mar 8, 2021

Merge branch 'master' into TIMOB-28376

dc6cc2c

garymathews changed the title ~~fix(android): fragments in children of header-/footerview~~ fix(android): fragments in children of header/footerview Mar 8, 2021

jquick-axway and others added 4 commits March 9, 2021 14:16

Merge branch 'master' into TIMOB-28376

cd35b9a

Merge branch 'master' into TIMOB-28376

8c21f81

fix(android): release of ListView headerView/footerView

bc3e78a

fix(anrdoid): headerView/footerView not updating when dark/light them…

a2e7d88

…e changes

garymathews approved these changes Mar 9, 2021

View reviewed changes

jquick-axway modified the milestones: 10.1.0, 10.0.0 Mar 9, 2021

jquick-axway added the backport 10_2_X when applied, PRs with this label will get an auto-generated backport to 10_2_X branch on merge label Mar 9, 2021

jquick-axway modified the milestones: 10.0.0, 10.1.0 Mar 9, 2021

ssjsamir self-requested a review March 10, 2021 16:31

ssjsamir approved these changes Mar 10, 2021

View reviewed changes

sgtcoolguy merged commit 4803091 into tidev:master Mar 11, 2021

build added the 10_0_X backport failed label Mar 11, 2021

sgtcoolguy pushed a commit that referenced this pull request Mar 11, 2021

fix(android): fragments in children of list view and tableview header…

ef5c95d

…/footerview (#12524) specifically allow to add a MapView ("ti.map") inside headerView of Ti.UI.ListView Fixes TIMOB-28376

hansemannn deleted the TIMOB-28376 branch March 11, 2021 15:06

ewanharris removed 10_0_X backport failed backport 10_2_X when applied, PRs with this label will get an auto-generated backport to 10_2_X branch on merge labels Mar 16, 2021

hansemannn commented Jan 13, 2022

View reviewed changes

drauggres mentioned this pull request Jan 14, 2022

Android: Random crash with map view in Ti.UI.ListSection footer #13243

Closed

hansemannn mentioned this pull request Apr 15, 2022

Android: Crash related to fragments #13399

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(android): fragments in children of header/footerview #12524

fix(android): fragments in children of header/footerview #12524

hansemannn commented Mar 7, 2021 •

edited by jquick-axway

Loading

build commented Mar 7, 2021 •

edited

Loading

m1ga commented Mar 8, 2021

hansemannn commented Mar 8, 2021

garymathews left a comment

ssjsamir left a comment

build commented Mar 11, 2021

hansemannn Jan 13, 2022 •

edited

Loading

drauggres Jan 13, 2022

hansemannn Jan 13, 2022

drauggres Jan 14, 2022 •

edited

Loading

m1ga Jan 14, 2022 •

edited

Loading

m1ga Jan 14, 2022 •

edited

Loading

hansemannn Jan 14, 2022

m1ga Jan 14, 2022

hansemannn Jan 14, 2022

drauggres Jan 14, 2022

fix(android): fragments in children of header/footerview #12524

fix(android): fragments in children of header/footerview #12524

Conversation

hansemannn commented Mar 7, 2021 • edited by jquick-axway Loading

build commented Mar 7, 2021 • edited Loading

Tests:

m1ga commented Mar 8, 2021

hansemannn commented Mar 8, 2021

garymathews left a comment

Choose a reason for hiding this comment

ssjsamir left a comment

Choose a reason for hiding this comment

build commented Mar 11, 2021

hansemannn Jan 13, 2022 • edited Loading

Choose a reason for hiding this comment

drauggres Jan 13, 2022

Choose a reason for hiding this comment

hansemannn Jan 13, 2022

Choose a reason for hiding this comment

drauggres Jan 14, 2022 • edited Loading

Choose a reason for hiding this comment

m1ga Jan 14, 2022 • edited Loading

Choose a reason for hiding this comment

m1ga Jan 14, 2022 • edited Loading

Choose a reason for hiding this comment

hansemannn Jan 14, 2022

Choose a reason for hiding this comment

m1ga Jan 14, 2022

Choose a reason for hiding this comment

hansemannn Jan 14, 2022

Choose a reason for hiding this comment

drauggres Jan 14, 2022

Choose a reason for hiding this comment

hansemannn commented Mar 7, 2021 •

edited by jquick-axway

Loading

build commented Mar 7, 2021 •

edited

Loading

hansemannn Jan 13, 2022 •

edited

Loading

drauggres Jan 14, 2022 •

edited

Loading

m1ga Jan 14, 2022 •

edited

Loading

m1ga Jan 14, 2022 •

edited

Loading