Spurious diffs when scrollbar is visible

[lencioni]

I have a component that adds its own scrollable container within the document. This causes the scrollbar to be part of the component, and therefore, part of the screenshot. For some reason, the scrollbar is sometimes a slightly different color, which causes a spurious diff. This is in Travis CI.

One thought is that the scrollbar is animated here, so we are catching it at a slightly different frame.

[trotzig]

Do you think we could fix this by overriding the scrollbar styling? I think you can do that, at least for firefox.

On Thursday, 1 September 2016, Joe Lencioni notifications@github.com wrote:

I have a component that adds its own scrollable container within the document. This causes the scrollbar to be part of the component, and therefore, part of the screenshot. For some reason, the scrollbar is sometimes a slightly different color, which causes a spurious diff. This is in Travis CI.

[image: scrollbar] https://cloud.githubusercontent.com/assets/195534/18179418/33b6b616-7037-11e6-9ecf-75a4a643d03e.png

One thought is that the scrollbar is animated here, so we are catching it at a slightly different frame.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Galooshi/happo/issues/131, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjS5eNCS4hGXMt_ca4t7ydVQe6VhBi6ks5qlxm2gaJpZM4Jy_cd .

[lencioni]

Yeah, that's what I was thinking, although it is not the most convenient to try out things because the scrollbars are all hidden by default on the OS that I'm on.

[lencioni]

Also, I've been having a hard time finding resources on styling scrollbars in Firefox. I found this open bug from 2001 though: https://bugzilla.mozilla.org/show_bug.cgi?id=77790

[lencioni]

I wonder if there is a way to identify a portion of a node as having a scrollbar in it. We could use that to determine if there is a scrollbar at the right edge, and then crop that part out. Maybe using https://developer.mozilla.org/en-US/docs/Web/API/Document/elementFromPoint ?

Alternatively, what do you think about finding any element that has overflow: scroll and re-style it to be overflow: hidden? I think this would be safe, but I can't think of a fast way to determine elements that might have scrollbars--I think you would need to either getComputedStyle of every node or compare scrollWidth/scrollHeight to clientWidth/clientHeight on every node (maybe that's not too bad).

[lencioni]

I whipped up a quick POC, and with 10,000 nodes, checking the scroll dimensions against the client dimensions adds essentially no additional timetime in addition to checking the bounding client rectangle (which we already do for every node). I think this might be a good solution and I'll submit a PR shortly.

[lencioni]

One downside to this approach is that if we want to run visual regression testing on different browsers and platforms, it can be useful to see that there is a scrollbar sometimes, because that can affect the widths of elements, which may affect how things flow or line up. I'm going to not worry about this for now though.

[lencioni]

This was a noble attempt, but unfortunately, it did not actually work for us.

[lencioni]

It is possible that this is actually working, but the browser just hasn't had an opportunity to repaint yet. That's the hypothesis that I will test next.

[lencioni]

After looking at this more closely for this case, I have determined that this is happening because this component happens to use a portal to render its contents in a different node. In this case, we are using the snapshotEntireScreen option to take a screenshot of the whole screen. Not only does this bypass the entire code tree that has the scrollbar disabling logic in it, it also wouldn't work with that logic because that only affects nodes within the tree starting with the node returned by the example--but in this case, all of the DOM nodes are outside of that tree.

I think a good way to solve this would be to change how Happo works a little. Right now each example has to return a reference to a DOM node, which doesn't work so well for things that render in two parts of the DOM (e.g "portals" in React apps). Instead of doing this, I think we should use our getFullRect routine on all of the nodes in document.body. This would likely work for every case, simplify the API, and allow us to remove the snapshotEntireScreen option.