Galooshi / happo

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

<option> elements can make screenshots too tall #180

Open trotzig opened 7 years ago

trotzig commented 7 years ago

We have an example that renders a "Birthdate" selection list. This example ends up having a full rect of about five times the height of the screen. This is because <option> elements report being offset from the top by a lot.

This is an example output from logging the getBoundingClientRect() value of a bunch of <option> elements:

<option value="1905"> DOMRect { x: 455.75, y: 2611, width: 182.25, height: 21, top: 2611, right: 638, bottom: 2632, left: 455.75 } 
<option value="1904"> DOMRect { x: 455.75, y: 2632, width: 182.25, height: 21, top: 2632, right: 638, bottom: 2653, left: 455.75 } 
<option value="1903"> DOMRect { x: 455.75, y: 2653, width: 182.25, height: 21, top: 2653, right: 638, bottom: 2674, left: 455.75 } 
<option value="1902"> DOMRect { x: 455.75, y: 2674, width: 182.25, height: 21, top: 2674, right: 638, bottom: 2695, left: 455.75 }

In Chrome, these are reported differently:


VM534:1 <option value=​"1889">​1889​</option>​ ClientRect {top: 0, right: 0, bottom: 0, left: 0, width: 0…}
VM534:1 <option value=​"1888">​1888​</option>​ ClientRect {top: 0, right: 0, bottom: 0, left: 0, width: 0…}
VM534:1 <option value=​"1887">​1887​</option>​ ClientRect {top: 0, right: 0, bottom: 0, left: 0, width: 0…}

This isn't a terrible problem in v3, because the resulting screenshot will be smaller. But in v4 (beta) we're constructing a screenshot box with the full dimensions. Which makes us end up with snapshots that look like this: http://localhost:4567/resource?file=snapshots%2FPEFicmlkZ2VkQWNjb3VudFNpZ25VcEZvcm0%2BIGRlZmF1bHQ%3D%2F%40medium%2Fcurrent.png

lencioni commented 7 years ago

You linked to localhost, so I can't see the image. Also, is this a duplicate of #136?

trotzig commented 7 years ago

Lol, I wrote that up a little bit too quickly.

It's sort of related to that, but I wonder if we should just check for visibility and ignore if invisible.

trotzig commented 7 years ago

This is the snapshot image. It's actually a little taller than what you see in this screenshot.

screen shot 2017-01-10 at 09 47 40