search

oliyh / kamera

UI testing via image comparison and devcards
91 stars 7 forks source link

Get width and height from JavaScript evaluation. #30

Closed jleonard-r7 closed 3 years ago

jleonard-r7 commented 3 years ago

Fixes #29

oliyh commented 3 years ago

Hi,

Thanks for this! I would like to keep kamera.core agnostic of devcards, so this could better be exposed as a key in the :resize-to-contents options map (called :dom-selector or something, defaulting to body in core but overridden to the devcards selector in kamera.devcards). This would ensure users could retain the same behaviour if they wished as well.

oliyh commented 3 years ago

This is also something I would like to test on my projects to see what difference it would make

jleonard-r7 commented 3 years ago

Makes sense. Ok, I will add the :dom-selector change to this PR and update accordingly.

oliyh commented 3 years ago

Sorry I don't think I made myself clear, I think the :dom-selector should be inside the :resize-to-contents map, e.g. {:resize-to-contents {:width? true :height? true :dom-selector "body"}} as they all relate to the same feature (keeps the fn signatures cleaner too)

jleonard-r7 commented 3 years ago

Ah I agree. That makes sense. Will update later.

jleonard-r7 commented 3 years ago

Is this ready to merge now?

oliyh commented 3 years ago

Hi,

Thanks for the updates. I am just going to try this build with my existing projects to see what happens as I am curious about how the javascript size and devcards selector will behave, and also to see what existing users can expect when they upgrade. I'll try to do this tomorrow.

Cheers

jleonard-r7 commented 3 years ago

Ok, with default settings, the only change they should see would be that the bug is actually fixed; i.e., the screenshots will be sized to content in the vertical direction. That will likely break tests where their content exceeded the vertical extent of one page (but that's most likely a good thing). They will either need to update their reference images or change the config to disable sizing to content in the vertical direction.

Everything else should be as expected / normal / least surprise.