Galooshi / happo

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

Disable nested scrollbars before taking screenshots #134

Closed lencioni closed 8 years ago

lencioni commented 8 years ago

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.

I'm not entirely sure why the scrollbar is sometimes different. Regardless, I don't think it is all that useful to have the scrollbar as part of the screenshot.

I came up with the idea to check each node and see if it has a scrollbar by comparing the scroll dimensions to the client dimensions. If it does, we simply re-style the node to be overflow: hidden. In a proof-of-concept with 10,000 nodes, adding this check to the traversal and bounding client rectangle check adds essentially no extra time, which sounds good to me.

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 since we aren't very close to being in this world anyway.

I believe that this fixes #131.

I am also using Aphrodite in a project which adds styles to the head that all include !important. This works well, except it overrides our animation-disabling overrides. I think if this scrollbar fix is successful, we might want to apply the same technique to animations.

trotzig commented 8 years ago

LGTM!