titanium-sdk icon indicating copy to clipboard operation
titanium-sdk copied to clipboard

feat(android, ios): continuous update for ListView scrolling event

Open m1ga opened this issue 3 years ago • 10 comments

JIRA: https://jira.appcelerator.org/browse/AC-6722

When continuousUpdate is enabled the scrolling event will fire for every visibleItem/Section change.

Test:

  • start example app
  • start scrolling
  • in the top right corner (or log) you'll see the current index/section
  • click "change" button to enable/disable continuousUpdate

disabled or with 10.1.1GA you only see an update when you change scroll direction.

Example

var toggle = true;
var sections = [];
const win = Ti.UI.createWindow();
const btn = Ti.UI.createButton({
	title: "change",
	bottom: 20
});
const lbl = Ti.UI.createLabel({
	top: 10,
	right: 10,
	width: Ti.UI.SIZE,
	height: Ti.UI.SIZE
})
const listView = Ti.UI.createListView({
	height: Ti.UI.FILL,
	width: Ti.UI.FILL,
	continuousUpdate: true
});

btn.addEventListener("click", function() {
	toggle = !toggle;
	listView.continuousUpdate = toggle
	console.log(listView.continuousUpdate);
})

for (var s = 0; s < 5; s++) {
	var section = Ti.UI.createListSection({
		headerTitle: 'Section ' + s
	});
	var set = [];
	for (var i = 0; i < 20; ++i) {
		set.push({
			properties: {
				title: 'Item 0 ' + i
			}
		})
	}
	section.setItems(set);
	sections.push(section);
}

listView.sections = sections;
win.add([listView, btn, lbl]);

listView.addEventListener("scrolling", function(e) {
	lbl.text = "Item: " + e.firstVisibleItemIndex + " section:" + e.firstVisibleSectionIndex;
	console.log("Item", e.firstVisibleItemIndex, e.firstVisibleSectionIndex);
})
win.open();

m1ga avatar Sep 30 '21 10:09 m1ga

Warnings
:warning:

Commit fe959f43eb80b1e0e8ff7167f2b5f54aa38c7df6 has a message "still fixing linter" giving 2 errors:

  • subject may not be empty
  • type may not be empty
:warning:

Commit 481a6d742ddd94d9499c6f6ad009ae18efb476ea has a message "remove no-unused-vars" giving 2 errors:

  • subject may not be empty
  • type may not be empty
:warning:

Commit 07df3d97eee757033527ff1f5289747c0fea6b6b has a message "add test" giving 2 errors:

  • subject may not be empty
  • type may not be empty
:warning:

Commit 8d3bf774fb0809cdaa61ccb412b03cc50ed0a2fd has a message "add test" giving 2 errors:

  • subject may not be empty
  • type may not be empty
:warning:

Commit c8fc0c92e2e551f005999f9f2f8196ed2b0ec7c5 has a message "add test" giving 2 errors:

  • subject may not be empty
  • type may not be empty
:warning:

Commit fddae5a8079ac5a3e27d5641be67a96ea7d3aded has a message "fix readme" giving 2 errors:

  • subject may not be empty
  • type may not be empty
:warning:

Commit d93f6f729e05f783945a9b50e275e352ab29cf43 has a message "fix linting" giving 2 errors:

  • subject may not be empty
  • type may not be empty
:warning:

Commit ae7bf882b45c155009aedaeefcf0d9e7794d9a05 has a message "fix linting" giving 2 errors:

  • subject may not be empty
  • type may not be empty
:warning: Tests have failed, see below for more information.
Messages
:book:

:rotating_light: This PR has one or more commits with warnings/errors for commit messages not matching our configuration. You may want to squash merge this PR and edit the message to match our conventions, or ask the original developer to modify their history.

:book: :x: 4 tests have failed There are 4 tests failing and 1157 skipped out of 20816 total tests.
:book: :tada: Another contribution from our awesome community member, m1ga! Thanks again for helping us make Titanium SDK better. :thumbsup:
:book:

:floppy_disk: Here's the generated SDK zipfile.

Tests:

ClassnameNameTimeError
android.emulator.5.0.Titanium.UI.View"after all" hook for "rgba fallback" (5.0.2)20.331
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (app.js)
android.emulator.5.0.Titanium.UI.View"after each" hook for "getOrCreateView() should always return a View" (5.0.2)10.315
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (app.js)
android.emulator.main.Titanium.UI.ImageView.image with Ti.Blob PNG (12)0.202
Error: expected false to be true
at Assertion.fail (/node_modules/should/cjs/should.js:275:13)
      at Assertion.value (/node_modules/should/cjs/should.js:356:9)
      at ImageView.listener (/ti.ui.imageview.test.js:177:39)
      at ImageView.value (ti:/kroll.js:1604:27)
      at ImageView.value (ti:/kroll.js:1656:25)
android.emulator.main.Titanium.UI.ListViewscrolling event (12)5.005
Error: Timeout of 5000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (app.js)

Generated by :no_entry_sign: dangerJS against 86fe0d8fbfad16bd5f904ad09135ac3ef45e479f

build avatar Sep 30 '21 11:09 build

@m1ga , did you run clang-format on the Java code?

You don't need to do that anymore. We use Java "checkstyles" which will trigger a build error for anything that violates our coding guidelines and explain what the issue is. Also, Android Studio's build log will show a clickable link that will take you directly to the line of code that violated the rule.

jquick-axway avatar Sep 30 '21 22:09 jquick-axway

Thanks @jquick-axway. I saw those warnings e.g. line too long and then I thought I'll run clang to automatically fix those warnings. Didn't know that it's not needed anymore. Was a quick automatic way of fixing all issues. I've fixed the unneeded changes

m1ga avatar Oct 01 '21 08:10 m1ga

add a test. If it is not enabled it will only fire scrolling once at the start so the count is 1. With the property it (should) be around 90, so I check if it is >50

m1ga avatar Oct 23 '21 16:10 m1ga

@hansemannn could you have a look at this one.

m1ga avatar Mar 25 '22 14:03 m1ga

The code looks fine - all additions to the code are guarded / in separate parts. Same for iOS. Have you tested iOS properly as well? And for custom cells?

hansemannn avatar Mar 25 '22 16:03 hansemannn

In any case, I would prioritize bug fixes first at this stage and do features / improvements after that, with a prio on features that have full parity.

hansemannn avatar Mar 25 '22 16:03 hansemannn

tested in on iOS too with the example and an app with a ListView with 4 templates, 5 sections. And since its an older PR the whole iOS/Android test-suite was done too.

The new parts are hidden behind a false by default property so it won't affect any existing apps. Only if you set continuousUpdate:true it will make any difference. But it can wait for other PRs :smile:

m1ga avatar Mar 25 '22 18:03 m1ga

I feel like it's not the most frequently asked-for feature, so let's approve it already, but merge after the fixes have all been merged.

hansemannn avatar Mar 25 '22 18:03 hansemannn

Sounds good. It comes very handy if you'll need to change a section/menu depending on your scroll position and for lazy loading. Markers are skipped sometimes when you add them dynamically in different spots.

m1ga avatar Mar 25 '22 18:03 m1ga

@m1ga Happy to take this before 12, e.g. 11.1.0!

hansemannn avatar Aug 18 '22 12:08 hansemannn

@m1ga Happy to take this before 12, e.g. 11.1.0!

ah nice! Wasn't sure :smile: It's covered by a test case too so from a functional state point quick to test/merge

m1ga avatar Aug 18 '22 12:08 m1ga