Galooshi / happo

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

Using all DOM nodes when snapshotting makes it harder to control width of snapshot #148

Closed trotzig closed 8 years ago

trotzig commented 8 years ago

The fact that we now iterate through all elements to get the bounding box to screenshot means that it's much harder to control the width of the resulting snapshot image. We use React to render, which means we create a div as the react root, and then render our component(s) in this root. Before, we'd just make sure that the examples returned the inner nodes (direct child of react root), but now we can't do that. Which means that it's the bounding rectangle of the react root div that's used when snapshotting.

First, we actually ran into an issue with collapsing margins due to how the react root wasn't creating a new block formatting context: https://github.com/Galooshi/happo/issues/145

I managed to fix that by applying a clear-fix (:after { content: ' '; display: table } to the react root. I explored other ways of avoiding the collapsing margin issue, but they all came with their own gotchas. display: inline-block would prevent full-width snapshots, but made it a lot harder to render inner content that was meant to go full width.

Right now I'm going to explore using the <body> as the react root, although I expect that to come with some pretty weird side-effects.

trotzig commented 8 years ago

@lencioni and I talked about this in a chat and we decided to add a happo.getRootNodes function in order to fix this: 34926914d4a3.