Galooshi / happo

Visual diffing in CI for user interfaces
505 stars 16 forks source link

Include all nodes in screenshots #141

Closed lencioni closed 8 years ago

lencioni commented 8 years ago

I've been looking into a case where we get spurious diffs on a scrollbar added to an element. In 239611300f I added some code that disabled scrollbars on nested elements, but this didn't work for my particular case.

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.

Fixes #131.