PolymerElements / iron-list

Element for a virtual, "infinite" list
https://www.webcomponents.org/element/PolymerElements/iron-list
219 stars 131 forks source link

Focus doesn't restore correctly after focused item is scrolled off screen. #537

Closed shc023 closed 6 years ago

shc023 commented 6 years ago

Description

If an item in the list is focused and then scrolled off screen, when you tab to focus the list again it does not correctly focus that item.

Expected outcome

if an item had focus, when the list is focused again the item should be focused correctly even if its scrolled off screen.

Actual outcome

the list scrolls back around that item but there is no focus.

Live Demo

http://jsfiddle.net/pohsybwx/10/

Steps to reproduce

See demo fiddle

Additional information

I did quite a bit of code tracing, and the most I could find out is the following: When trying to focus the list again, the code path goes: didFocus() --> scrollToIndex() --> _manageFocus() --> _restoreFocusedItem() and when it got to this line: https://github.com/PolymerElements/iron-list/blob/master/iron-list.html#L1816 the items in the if are expected to match, but it does not. This causes it to go into the else block, triggering removeFocusedItem() and messing up the focus. The onScreenInstance and offScreenInstance always seems to be a few indexes off, if you compare their __key__ value.

I went through the same scenario with our previous version (v1.4.6) and the equivalent if check here: https://github.com/PolymerElements/iron-list/blob/v1.4.6/iron-list.html#L1831 the items did match and focus is restored correctly, so there seems to be a regression.

shc023 commented 6 years ago

I tried the fix locally in our code base and it fixed the issue. Looking forward to patching the next release, thanks!