-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(android): fix Ti.UI.ListView scroll state restoration #13493
fix(android): fix Ti.UI.ListView scroll state restoration #13493
Conversation
some kind of test code would be very nice next time to test it. I've quickly created this one: const win = Ti.UI.createWindow({});
var listView = Ti.UI.createListView({
templates: {
test: {
childTemplates: [{
type: 'Ti.UI.View',
childTemplates: [{
type: 'Ti.UI.Label',
bindId: 'label',
properties: {
color: 'black',
bindId: 'label',
bottom: 5
}
}],
properties: {
width: Ti.UI.FILL,
height: 100,
cardUseCompatPadding: true,
backgroundColor: 'white'
}
}]
}
},
defaultItemTemplate: 'test'
});
var section = Ti.UI.createListSection();
var items = [];
function addItems(adding) {
for (let i = 0; i < 100; i++) {
var txt = "Row: " + (i + adding);
items.push({
label: {
text: txt
},
template: 'test'
})
}
section.setItems(items);
}
listView.sections = [section];
win.add(listView);
win.addEventListener('open', function() {
addItems(0);
setTimeout(function(){
listView.scrollToItem(0,50);
setTimeout(function(){
addItems(100)
}, 2000);
}, 1000)
})
win.open(); Without the PR it will scroll to the top after 3 sec. With the PR it will stay at the same position and just attach the items. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine, test code works better with the PR and my existing app still works the same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No scrolling detected with this fix
LGTM
We have to revert it I think, the PR was not finished so far. The potential open issue with the missing focus needs to be tested on device. Sorry for the missing code @m1ga, I was about to add it this evening. Its already happening when generating a long list of items and setting the sections again (causing the internal |
@hansemannn I've tested it on a device (Pixel 4, Android 12) and emulator (Android 13). |
Okay, so the view did not lose focus / turn gray? Then it's all good. |
Minimalistic test case: const window = Ti.UI.createWindow();
const listView = Ti.UI.createListView({
sections: [
Ti.UI.createListSection({
items: generateItems()
})
]
});
window.add(listView);
window.open();
setTimeout(() => {
listView.sections[0].items = generateItems();
}, 3000);
function generateItems() {
return Array.from(Array(30).keys()).map(num => ({ properties: { title: `Cell ${num + 1}` } }))
} |
I guess we are fine then. Cool! |
not sure what you mean by that 😄 Your example looks the same with 11.0.0.RC and the patched SDK on my device, at start and 3 seconds later 😄 It only fixes the scrolling that my example is showing. Rest looks fine |
In my internal example, we have a text field in the item and after hitting the "Return" key, the list seems to lose focus. But it seems okay. |
One note: At least on the Emulator, the list view seems to lose focus after re-rendering the list view. Not sure if thats an Emulator-only issue.