Viewport for scenarios

[kiran-redhat]

Hey @garris

It's a great tool, appreciate your efforts on this.

I am testing with 8 viewports and the challenge I had is there are different selectors for different viewports(selector changes and different layout gets displayed based on viewport size). I have a one onReadyScript to navigate to that particular screen(where layout gets changed) then scenario has a selector to capture screen. It can't find the selector(as it is changed for mobile vieport) while looping through each viewport then takes no screenshot but displays the default "selectorNotFound_noun_164558_cc.png". With this PR, we can specify same onReadyScript file and have two scenarios with different selectors (one for laptop size & other for mobile size) so it won't show 'selector not found image".

Alternative to this would be using two onReadyScript scripts but we have to repeat the same code & selectors twice.

With this PR: Scenario will use it's viewports if exists. This is useful when testing responsive websites. For e.g. selector may change based on viewport's size for one or two scenarios in which case you don't want to create another config file.

[djskinner]

I believe I have a requirement for this functionality too. I'm working with display screens (think advertising display boards) and the URL parameters are actually tied to the possible viewport sizes.

[kiran-redhat]

@garris , would you like to have a look at this PR?

[garris]

I see where you're going with this but I am concerned about this change. I think this could cause a regression with getFilename() in genBitmaps.js. There was a lot of effort put into ensuring there could not be any filename collisions -- I am not sure this change is compatible with that.

Unfortunately there are no tests around that yet. If you wanted to pursue this change you'd need to create some tests to ensure both these two issues are ok. Also, there is a lot of work going into the chrome-headless branch. You would need to add this change there also.

In the past, for cases like this I usually recommend just creating multiple configs and running them from the command line or a runner script -- just create a config for each viewport.

[kiran-redhat]

I see the need to add tests for getFilename() in genBitmaps.js.

Using different config files for different view ports is good, however the challenge with that is maintaining two sets, and some times selector may be same for these different view ports in which case we will end up repeating same scenario.

[djskinner]

I'm still wondering how best to approach this.

In the past, for cases like this I usually recommend just creating multiple configs and running them from the command line or a runner script -- just create a config for each viewport.

For my current requirement this would be say 10 different configs, each with one scenario. This is because the URL and the viewport are coupled.

Is it possible to combine the backstop results into a single report or would I have to work with 10 separate reports?

Can you point to any example code where multiple configs have been employed?

[AkA84]

is there any update on this? This is something that in our project could really be useful

[garris]

Many apologies to everyone on this thread. This change was posted when I was overwhelmed with other work. If anyone wants to pick this up and test it we could work to bring it in.

[SengitU]

I also need this feature, how can I help this PR?

[kiraarghy]

As with @SengitU this would be incredibly useful for me too. Can I help at all?

[garris]

Ok. It has come to my attention that people really really want this update. Backstop has changed very significantly since the original pr for this. For anyone willing to refactor this for puppeteer I would gladly bring this in. We just need to have sufficient tests around it. Just to be clear about the api — if backstop finds a viewports array inside a scenario, that one would be used instead of the one at the root of the config. Does that work?

[kiraarghy]

@garris I've written a rough new implementation of this, will submit it shortly.

Could I ask:

• Why would this need different behaviour for Puppeteer, Chromy and Puppeteer seem to use the scenarioViews which include a viewport size?

• Regarding tests, can't see it in the contributions guidelines but would you like unit tests to cover the changed functionality too, or just smoke tests?

[garris]

That’s great!

1. ideally it is the same implementation for Chromy and puppeteer. I haven’t been in there for a bit so I am not sure if they need different implementation.

2. please add to the smoke tests— we want to see that those are working as expected.

[SengitU]

Hello,

I've created a PR for this feature, feel free to share your comments. See #844 .