Bugzilla – Full Text Bug Listing
| Summary: | [List] Scrolling with topIndex does not work if items are loaded at the same time | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [RT] RAP | Reporter: | Claudio Guglielmo <claudio.guglielmo> | ||||
| Component: | RWT | Assignee: | Project Inbox <rap-inbox> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P2 | CC: | ivan, tbuschto | ||||
| Version: | 1.5 | ||||||
| Target Milestone: | 2.1 RC1 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
Actually, the List already accounts for this scenario with this code:
_onAppear : function( evt ) {
// [ad] Fix for Bug 277678
// when #showSelection() is called for invisible widget
this._applyTopIndex( this._topIndex );
},
Not sure yet why it still doesn't work correctly.
Also, please attach patches and snippets uncompressed. That makes it much easier to take a look at them quickly. Thanks.
(In reply to comment #1) > Actually, the List already accounts for this scenario with this code: > > _onAppear : function( evt ) { > // [ad] Fix for Bug 277678 > // when #showSelection() is called for invisible widget > this._applyTopIndex( this._topIndex ); > }, > > Not sure yet why it still doesn't work correctly. > > Also, please attach patches and snippets uncompressed. That makes it much > easier to take a look at them quickly. Thanks. I think that the problem exists when the List is already created on the client and you you change items and topIndex in a single request. Items are rendered before topIndex in ListAdapeter.js, but maybe a flush of new created DOM elements is needed to correctly respect the topIndex change. Fixed with commit f1ca48b62412473a984566c0f8f25b4c39ecab73. The problem was that the scroll position has to be applied to the html-element AFTER the items have already been rendered, which happens in the display-flush. This had to be fixed on several levels: 1. The position value has to be applied to the scrollbar, not the clientArea directly, but after the scrollbars maximum has been updated. 2. Doing so triggers _syncClientArea in Scrollable.js, which now detects that the position could not be set yet, an schedules it to be applied AFTER the next flush. I'm hesitant to apply this to 1.5, as it's a relatively big change. On the other hand it's mostly additional code that can't really do any harm. In a discussion with Tim, we agreed not to include this fix in the 1.5.2 service release. It overrides methods of Scrollable and involves a slight risk of breaking something else. The bug only affects code that uses a certain pattern - adding an item and immediately setting the top index of a List. As a workaround, the top index can be set in a timerExec in 1.5. Thanks for solving the bug, Tim! I did a little test and it works way better, but still not perfect. Run the already attached example and scroll a little bit upwards. Then press on the button "add more elements and scroll to bottom". The elements are added but it only scrolls to the former last element, which is Item 99 instead of Item 119. (In reply to comment #5) Indeed. It looks like at some point Scrollable.js#_internalChangeFlag is set to the wrong value (false), and the desired scroll position is lost for that reason. The whole native-html scrolling thing is a nightmare. Using "top" (or css transform where possible) instead of "scrolltop" would be so much simpler. The only reason we don't use it is because some input methods for scrolling (touch and scroll wheel) would have to be implemented explicitly, and might not behave as the user is used to. Perhaps it's worth the effort anyway. I must admit I am not very familiar with the Scrollable.js so I can't help you with that. But I just did the same test with rap 1.5 and my little workaround in _applyTopIndex and it seems to work. Maybe this info helps...
var func = function() {
if(this._clientArea != null) {
this._clientArea.setScrollTop( newIndex * itemHeight );
}
};
qx.client.Timer.once( func, this, 60 );
It looks like the issue is in the ScrollBar. The scroll position may be changed during an internal layouting process: console.trace() rap-client.js:58967 rwt.qx.Class.define.members._syncClientArea rap-client.js:58967 rwt.qx.Class.define.members._onVertScrollBarChangeValue rap-client.js:58929 rwt.qx.Class.define.members._dispatchEvent rap-client.js:6260 rwt.qx.Class.define.members.dispatchEvent rap-client.js:6190 rwt.qx.Class.define.members.createDispatchEvent rap-client.js:6124 rwt.qx.Class.define.members._dispatchValueChanged rap-client.js:35740 rwt.qx.Class.define.members._updateStepsize rap-client.js:35747 rwt.qx.Class.define.members._updateThumbSize rap-client.js:35391 rwt.qx.Class.define.members.base rap-client.js:5180 rwt.qx.Class.define.members._updateThumbSize rap-client.js:35646 rwt.qx.Class.define.members._setThumb rap-client.js:35134 rwt.qx.Class.define.members._layoutPost rap-client.js:35672 rwt.qx.Class.define.members._layoutChild rap-client.js:16725 rwt.qx.Class.define.members.flushChildrenQueue rap-client.js:19462 rwt.qx.Class.define.members._flushChildrenQueue rap-client.js:16663 rwt.qx.Class.define.statics.flushGlobalLayoutQueue rap-client.js:9246 rwt.qx.Class.define.statics.flushGlobalQueues rap-client.js:9087 rwt.qx.Class.define.members._handleSuccess rap-client.js:69685 _success rap-client.js:36179 rwt.remote.Request._onReadyStateChange rap-client.js:36230 wrap Fixed (again) with commit 8be427df5f996449f9ccb021229d53828e3c27cd. There were multiple issues, but the main one was that some browser fire the onScroll event synchronously, most asynchronously. It's not consistent in IE. The code assumed it was fired synchronously. Also, ScrollBar did not apply "ideal" values (remembered) values correctly and List did not update scroll position correctly if item dimension changed. |
Created attachment 223942 [details] Example, images and workaround (patch) I'd like to scroll to the bottom of the list. To do this, I create a list and set topIndex to itemCount -1. But: It doesn't scroll. If I set topIndex on a list which already contains items, it works fine. A little debugging showed, that the scroll position is set before the items are actually rendered. See the attached snippet for example code. The attachment also contains a workaround as patch which makes it better but still not perfect. See the images for details. If I use a table, it works. I assume because of the following code: if( this._inServerResponse() ) { this._scheduleUpdate( "topItem" ); } else { .. } Probably something like that needs to be done for the list too.