showOnScreen doesn't trigger scroll if item is already fully on screen - #17729

can you assert leadingEdgeOffset <= trailingEdgeOffset?

No, but I can guarantee it the other way around: leadingEdgeOffset >= trailingEdgeOffset :)

(really? that's surprising)

in that case, assert that then simplify the expressions

To get a list item to the leading edge (as opposed to the trailing edge) you have to scroll further, therefore leadingEdgeOffset >= trailingEdgeOffset, no?

Will simplify expression.

In a regular vertically scrolling list, the leading edge is above the trailing edge, so you get it first (lower scroll offset) than the trailing edge (higher scroll offset) unless I'm missing something?

To ensure that we talk about the same thing I have drawn up two diagrams: Both show a long scrollable list (on the left) that scrolls through the viewport window (on the right in yellow).

In the first picture, the list item in question is shown at the leading edge of the viewport. I'd argue that the scrollOffset here is 600, which is called the leadingEdgeOffset in the code.

Now, let's move the list item in question to the trailing edge of the viewport. I'd argue that in this picture the scrollOffset is 350, which is called the trailingEdgeOffset in the code.

So, now I would argue that 600 > 350. And therefore (even though this is by now means a full mathematically proof), it seems that leadingEdgeOffset >= trailingEdgeOffset holds.

ah, yup, i was missing something. :-)

any chance we can factor the common code into the base class?

I've looked into doing that before and there is no good common ancestor for this...

create a common mixin (RenderViewportShowOnScreenMixin) or make a static method on some class (RenderViewportBase.showChildInViewport or some such) and use that.

Mixin doesn't work (or gets awkward because of the super call in the method. I changed it to a static method.

we should test that this works with negative-scroll-offset children too

Good idea. Added.