Check intersectionRatio to determine visibility correctly #25
Conversation
When i implemented originally I omitted checking intersectionRatio, but from Atom-v1.25.0, I noticed lots of changes with intersectionRatio === 0 reported, which break visible-on-render behavior. This PR correctly determine visibility and make render-on-visible work again.
|
|
||
| containerNode.appendChild(selectListView.element) | ||
|
|
||
| const children = selectListView.refs.items.children | ||
| children[20].scrollIntoViewIfNeeded() | ||
| await assertItemsBecomeVisible([17, 18, 19, 20, 21, 22, 23]) | ||
| await assertItemsBecomeVisible([18, 19, 20, 21, 22]) |
There was a problem hiding this comment.
Can you help me understand why these needed to change?
There was a problem hiding this comment.
17 and 23 here was "BecomeVisible" because by intersectionRatio === 0 not by intersectionRatio > 0, even in non-problematic Atom-v1.24.0.
So this is the spec created based on incorrect behavior. Now fixed incorrectly behavior, so need to fix spec too.
Just when I implemented it in Atom-v1.24.0, I didn't noticed this incorrectness.
[
17, # intersectionRatio == 0
18, # intersectionRatio == 1
19, # intersectionRatio == 1
20, # intersectionRatio == 1
21, # intersectionRatio == 1
22, # intersectionRatio == 1
23, # intersectionRatio == 0
]
To be precise
- Atom-v1.24.0(
Chrome : 56.0.2924.87) affect this bug, but not severe, it's just report border TWO invisible elements asintersectionRatio === 0. - Atom-v1.25.0-beta2(
Chrome: 58.0.3029.110) affect this bug severely, it's report ALL elements asintersectionRatio === 0.
I think this change comes from chrome version change by electron version up.
There was a problem hiding this comment.
Thanks for the explanation! Makes sense to me.
|
Thanks for merging this quickly! |
|
I'll see if I can get updates of atom-select-list and command-palette out today then get it bumped into the next 1.25.0-beta release. |
|
atom-select-list and command-palette have been updated and bumped in 1.25-releases and master! |
|
@daviwil Great! Thanks!! |
@leroix hope you check this.
When I implemented #22 originally, I omitted checking
intersectionRatio(at that time it seems for me ratio report is somewhat incorrect and judged OK with behavioral check), butfrom Atom-v1.25.0, I noticed lots of changes with
intersectionRatio === 0reported, which eventually callelementForItemwith{visible: true}, thus break visible-on-render behavior.This PR correctly determine visibility and make render-on-visible work again.
Impacted issue: atom/command-palette#108