titanium-sdk
titanium-sdk copied to clipboard
feat(android, ios): continuous update for ListView scrolling event
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();
Warnings | |
---|---|
:warning: |
Commit fe959f43eb80b1e0e8ff7167f2b5f54aa38c7df6 has a message "still fixing linter" giving 2 errors:
|
:warning: |
Commit 481a6d742ddd94d9499c6f6ad009ae18efb476ea has a message "remove no-unused-vars" giving 2 errors:
|
:warning: |
Commit 07df3d97eee757033527ff1f5289747c0fea6b6b has a message "add test" giving 2 errors:
|
:warning: |
Commit 8d3bf774fb0809cdaa61ccb412b03cc50ed0a2fd has a message "add test" giving 2 errors:
|
:warning: |
Commit c8fc0c92e2e551f005999f9f2f8196ed2b0ec7c5 has a message "add test" giving 2 errors:
|
:warning: |
Commit fddae5a8079ac5a3e27d5641be67a96ea7d3aded has a message "fix readme" giving 2 errors:
|
:warning: |
Commit d93f6f729e05f783945a9b50e275e352ab29cf43 has a message "fix linting" giving 2 errors:
|
:warning: |
Commit ae7bf882b45c155009aedaeefcf0d9e7794d9a05 has a message "fix linting" giving 2 errors:
|
: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:
Classname | Name | Time | Error |
---|---|---|---|
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.ListView | scrolling 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
@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.
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
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
@hansemannn could you have a look at this one.
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?
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.
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:
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.
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 Happy to take this before 12, e.g. 11.1.0!
@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