[TIMOB-13610] Android: Proposed fix for poor user experience when scrollableView is inside tableview/listview/scrollview

GitHub Issue	n/a
Type	Bug
Priority	Medium
Status	Closed
Resolution	Fixed
Resolution Date	2013-10-10T18:08:12.000+0000
Affected Version/s	n/a
Fix Version/s	2013 Sprint 21, 2013 Sprint 21 API, Release 3.2.0
Components	Android
Labels	android, exalture, listview, scrollableview, scrollview, tableview
Reporter	Martin Guillon
Assignee	Ping Wang
Created	2013-04-12T13:51:54.000+0000
Updated	2013-11-13T18:04:26.000+0000

Description

*Problem description* If you put a scrollableview inside a tableview/listview/scrollview, it s almost unusable as described in the docs: http://docs.appcelerator.com/titanium/3.0/#!/api/Titanium.UI.TableView {quote} As a table view inherently scrolls, it creates a very poor user experience when one contains other scrolling views, such as a ScrollableView or TextArea. Thus, this layout is strongly discouraged. {quote} The following pull request fixes that: *Pull request* https://github.com/appcelerator/titanium_mobile/pull/4153 *Steps to reproduce* 1. Run the reproduction test case. 2. See that scrolling the scroll view horizontally causes it to sometimes repel swiping. *Reproduction test case (any SDK)*

var win = Ti.UI.createWindow({
    backgroundColor : 'white',
    modal : false
});
 
var table = Ti.UI.createTableView();
var rowData = [];
 
var scrollable = Ti.UI.createScrollableView({
    top:0,
    left:0,
    bottom:0,
    right:0,
    backgroundColor:'orange',
    views : [Ti.UI.createView({
        backgroundColor : 'blue'
    }), Ti.UI.createView({
        backgroundColor : 'green'
    }), Ti.UI.createView({
        backgroundColor : 'yellow'
    })]
})
 
var row = Ti.UI.createTableViewRow({
    height : 120
});
row.add(scrollable);
rowData.push(row);
 
for (var i = 0; i < 40; i++) {
    rowData.push(Ti.UI.createTableViewRow({
        title : ("test2 " + i)
    }));
}
table.data = rowData;
 
win.add(table);
win.open();

*Pull request test case*

// open a single window

function tableviewtest() {
	var win = Ti.UI.createWindow({
		backgroundColor : 'white',
		modal : false
	});

	var table = Ti.UI.createTableView();
	var rowData = [];

	var scrollable = Ti.UI.createScrollableView({
		views : [Ti.UI.createView({
			backgroundColor : 'blue'
		}), Ti.UI.createView({
			backgroundColor : 'green'
		}), Ti.UI.createView({
			backgroundColor : 'yellow'
		})]
	})

	var row = Ti.UI.createTableViewRow({
		height : 120
	});
	row.add(scrollable);
	rowData.push(row);

	for (var i = 0; i < 40; i++) {
		rowData.push(Ti.UI.createTableViewRow({
			title : ("test " + i)
		}));
	}
	table.data = rowData;

	win.add(table);
	win.open();
}

function scrolliewtest() {
	var win = Ti.UI.createWindow({
		backgroundColor : 'white',
		modal : false
	});

	var scroll = Ti.UI.createScrollView({
		layout : 'vertical'
	});

	var scrollable = Ti.UI.createScrollableView({
		height : 120,
		views : [Ti.UI.createView({
			backgroundColor : 'blue'
		}), Ti.UI.createView({
			backgroundColor : 'green'
		}), Ti.UI.createView({
			backgroundColor : 'yellow'
		})]
	})

	for (var i = 0; i < 3; i++) {
		scroll.add(Ti.UI.createLabel({
			height : 80,
			text : ("test " + i)
		}));
	}
	scroll.add(scrollable);

	for (var i = 0; i < 40; i++) {
		scroll.add(Ti.UI.createLabel({
			height : 80,
			text : ("test " + i)
		}));
	}

	win.add(scroll);
	win.open();
}

function scrolliewtabletest() {
	var win = Ti.UI.createWindow({
		backgroundColor : 'white',
		modal : false
	});

	var scroll = Ti.UI.createScrollView({
		scrollType : 'horizontal',
		horizontalWrap : false,
		layout : 'horizontal'
	});

	var table = Ti.UI.createTableView({
		width : 800
	});
	var rowData = [];

	for (var i = 0; i < 40; i++) {
		rowData.push(Ti.UI.createTableViewRow({
			title : ("test " + i)
		}));
	}
	table.data = rowData;

	for (var i = 0; i < 3; i++) {
		scroll.add(Ti.UI.createLabel({
			width : 80,
			text : ("test " + i)
		}));
	}
	scroll.add(table);

	for (var i = 0; i < 40; i++) {
		scroll.add(Ti.UI.createLabel({
			width : 80,
			text : ("test " + i)
		}));
	}

	win.add(scroll);
	win.open();
}

function scrollabletest() {
	var win = Ti.UI.createWindow({
		backgroundColor : 'white',
		modal : false
	});

	var scroll = Ti.UI.createScrollableView({

	});

	var table = Ti.UI.createTableView({
		width : 300,
		left : 40,
		right : 40,
		backgroundColor : 'green'
	});

	var rowData = [];

	for (var i = 0; i < 40; i++) {
		rowData.push(Ti.UI.createTableViewRow({
			title : ("test " + i)
		}));
	}
	table.data = rowData;

	scroll.addView(table);

	var scroll2 = Ti.UI.createScrollView({
		layout : 'vertical'
	});

	for (var i = 0; i < 40; i++) {
		scroll2.add(Ti.UI.createLabel({
			height : 80,
			text : ("test " + i)
		}));
	}

	scroll.addView(scroll2);
	win.add(scroll);
	win.open();
}

var win = Ti.UI.createWindow({
	backgroundColor : 'white'
});
var tableview = Ti.UI.createTableView({
	rowHeight : 120,
	data : [{
		title : 'tableviewtest',
		callback : tableviewtest
	}, {
		title : 'scrolliewtest',
		callback : scrolliewtest
	}, {
		title : 'scrolliewtabletest',
		callback : scrolliewtabletest
	}, {
		title : 'scrollabletest',
		callback : scrollabletest
	}]
})
tableview.addEventListener('click', function(e) {
	if (e.rowData && e.rowData.callback)
		e.rowData.callback();
})
win.add(tableview);
win.open();

Comments

Martin Guillon 2013-04-12

pull request https://github.com/appcelerator/titanium_mobile/pull/4153
Daniel Sefton 2013-04-12

So I tried your test case with 3.1 CI on Samsung Galaxy S2 Android 2.3.6... It seems to work fine. Please could you add reproduction steps and describe exactly what the problem is? Or add a test case which reproduces your problem you described, not just testing your pull request. Thanks!
Martin Guillon 2013-04-13

test case to show the problem added
Daniel Sefton 2013-04-17

By unusable do you mean laggy? Again, I tried your new test case on Samsung Galaxy S2 Android 2.3.6 with 3.1 GA... It scrolls fine for me... I'll escalate anyway since you have a PR.

Martin Guillon 2013-04-18

@Daniel: First i am not sure why but something was making the scrollableview not appear when i just tested it. So see the updated test case bellow. Now i just tried again if you try using the scrollableview you ll see that changing page is a nightmare. THis is a know problem explain [here](http://docs.appcelerator.com/titanium/3.0/#!/api/Titanium.UI.TableView) With my PR everything should go smoothly even in a listview

   var win = Ti.UI.createWindow({
       backgroundColor : 'white',
       modal : false
   });
   
   var table = Ti.UI.createTableView();
   var rowData = [];
   
   var scrollable = Ti.UI.createScrollableView({
   	top:0,
   	left:0,
   	bottom:0,
   	right:0,
   	backgroundColor:'orange',
       views : [Ti.UI.createView({
           backgroundColor : 'blue'
       }), Ti.UI.createView({
           backgroundColor : 'green'
       }), Ti.UI.createView({
           backgroundColor : 'yellow'
       })]
   })
   
   var row = Ti.UI.createTableViewRow({
       height : 120
   });
   row.add(scrollable);
   rowData.push(row);
   
   for (var i = 0; i < 40; i++) {
       rowData.push(Ti.UI.createTableViewRow({
           title : ("test2 " + i)
       }));
   }
   table.data = rowData;
   
   win.add(table);
   win.open();

Daniel Sefton 2013-04-18

Thanks Martin - I have now confirmed what you described.
Sunila 2013-08-10

Looks like there is already a PR
Martin Guillon 2013-08-10

Yes there is a PR ;) but it s quite old now...
Hieu Pham 2013-09-19

We strongly discourage the use of tableview/listview inside a scrollview, as documented. It is a viable workaround for users who wish to do this, but I don't think we should be merging it into master. Closing issue.
Martin Guillon 2013-09-19

I cant believe what i am reading! You are actually refusing a PR which fixes a UI experience without any drawback? You know you are actually preventing Ti users to create UI like we see on native apps just because of that PR?
Dan Tamas 2013-09-19

@martin - don't get angry, not all the PRs are making it into the core, sometimes even if they look/are legit, however, @Hieu you DIDN'T READ what Martin says: there is a "scrollable INSIDE a tableRow", and not a "table inside a scrollview". We might need this sometimes, think on the the Appstore app, you can scroll horizontally to see the screenshots and vertically to see the content. This should be smooth on android as well. Just my 2c
Martin Guillon 2013-09-19

@Dan: you are right, i am not getting angry ;) (i might have answered too early in the morning though …) What bugs me is refusing a PR which improves interaction without drawback (that i can see since i have been using it :P) I can change my PR if necessary just to work with scrollable inside tableview and not the other way around (though i will keep it in my branch). Thanks Dan for your 2c
Ingo Muschenetz 2013-09-19

[~farfromrefuge] and [~rborn] One question I have, as suggested above, is that fixing this "has no drawbacks." I have certainly seen cases where fixing one issue can cause unintentended behavior changes or regressions elsewhere. How can we prove that's not the case?
Martin Guillon 2013-09-19

I cant prove it Ingo… BUT i have proof that it as NO effect when not using tableview inside scrollview or the other way. It s the implementation... So the edge case is only in a case you "discourage" (and still appears in a LOT of recent apps, aero, cinemur, yahoo weather,…) Finally that PR only mimic the way it s already implemented on iOS

Ping Wang 2013-09-27

[~farfromrefuge], your PR will introduce some regressions. For example, if we run the test case below with your fix, the table view is not scrollable at all. But it scrolls fine without your fix.

    var win = Ti.UI.createWindow({
    	backgroundColor : 'white',
    	modal : false
    });
    
    var table = Ti.UI.createTableView();
    var rowData = [];
    
    for (var i = 0; i < 10; i++) {
    	var scrollable = Ti.UI.createScrollableView({
    		top : 0,
    		left : 0,
    		bottom : 0,
    		right : 0,
    		backgroundColor : 'orange',
    		views : [Ti.UI.createView({
    			backgroundColor : 'blue'
    		}), Ti.UI.createView({
    			backgroundColor : 'green'
    		}), Ti.UI.createView({
    			backgroundColor : 'yellow'
    		})]
    	});
    
    	var row = Ti.UI.createTableViewRow({
    		height : 300
    	});
    	row.add(scrollable);
    	rowData.push(row);
    }
    
    for (var i = 0; i < 40; i++) {
    	rowData.push(Ti.UI.createTableViewRow({
    		title : ("test2 " + i)
    	}));
    }
    table.data = rowData;
    
    win.add(table);
    win.open();

Martin Guillon 2013-09-28

Thanks for pointing that out. I fixed it by implementing it another way New PR https://github.com/appcelerator/titanium_mobile/pull/4748

Ping Wang 2013-10-04

Two more test cases. ScrollableView inside scrollview:

    var win = Ti.UI.createWindow({
        backgroundColor : 'white',
        modal : false
    });
     
    var scrollView = Ti.UI.createScrollView({
      contentWidth: 'auto',
      contentHeight: 4000,
      showVerticalScrollIndicator: true
    });
    
    for (var i = 0; i < 10; i++) {
        var scrollable = Ti.UI.createScrollableView({
            top : 10*i + 300 *i,
            left : 0,
            bottom : 0,
            right : 0,
            height: 300,
            backgroundColor : 'orange',
            views : [Ti.UI.createView({
            	backgroundColor : 'blue'
            }), Ti.UI.createView({
            	backgroundColor : 'green'
            }), Ti.UI.createView({
            	backgroundColor : 'yellow'
            })]
        });
        
        scrollView.add(scrollable);
    }
     
    var view = Ti.UI.createView({
      backgroundColor:'#336699',
      borderRadius: 10,
      top: 3100,
      height: 200
    });
    scrollView.add(view);
    
    win.add(scrollView);
    win.open();

ScrollableView inside ListView:

    var win = Ti.UI.createWindow({backgroundColor: 'white'});
    
    var plainTemplate = {
        childTemplates: [
            {
                type: 'Ti.UI.ScrollableView',
                bindId: 'scrollable',
                properties: {
                    height: '300'
                }
            }
        ]
    };
    
    var listView = Ti.UI.createListView({
        templates: { 'plain': plainTemplate },
        defaultItemTemplate: 'plain'               
    });
    
    var data = [];
    for (var i = 0; i < 10; i++) {
        data.push({
            scrollable : { views : [Ti.UI.createView({
            	backgroundColor : 'blue'
            }), Ti.UI.createView({
            	backgroundColor : 'green'
            }), Ti.UI.createView({
            	backgroundColor : 'yellow'
            })] }
        });
    }
    
    var section = Ti.UI.createListSection({items: data});
    listView.sections = [section];
    
    win.add(listView);
    win.open();

Ping Wang 2013-10-07

Tested the PR with the three test cases in my comments. It works fine for ScrollableView inside ScrollView. It exposes some issues on ScrollableView inside TableView/ListView. Those issues are not caused by this PR. Already filed TIMOB-15408 to follow up.
Ping Wang 2013-10-10

PR: https://github.com/appcelerator/titanium_mobile/pull/4748
Samuel Dowse 2013-11-13

Verified fixed on: Mac OSX 10.9 Mavericks Titanium Studio, build: 3.2.0.201311122225 Titanium SDK, build: 3.2.0.v20131112144044 CLI: 3.2.0 Alloy: 1.3.0 Android Device: Xperia Arc S (4.0.4), Xperia U (2.3.3) All test cases used from description and comments. All work fine with no lag or other cause for "poor user experience". Closing.

JSON Source