Simplify Percy Breakpoint/Theme snapshots

[jmuzina]

Done

• Light examples are snapshotted responsively
• Dark/paper examples are snapshotted on desktop only
• Slightly simplified / cleaned up snapshots JS code by adding some constants to reduce literal duplication

Fixes WD-13804

QA

• View test build and verify snapshots are captured as expected
  • Pay special attention to the relationship between theme and breakpoints
    • All snapshots are captured at 1280px (desktop/large size)
    • Light-theme examples are also captured at 375px (mobile/small size)
    • Examples that include "responsive" in their name or embed such an example are also captured at 800px (tablet/medium size)

Check if PR is ready for release

If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:

• PR should have one of the following labels to automatically categorise it in release notes:
  • Feature 🎁, Breaking Change 💣, Bug 🐛, Documentation 📝, Maintenance 🔨.
• Vanilla version in package.json should be updated relative to the most recent release, following semver convention:
  • if CSS class names are not changed it can be bugfix release (x.x.X)
  • if CSS class names are changed/added/removed it should be minor version (x.X.0)
  • see the wiki for more details
• Any changes to component class names (new patterns, variants, removed or added features) should be listed on the what's new page.

Screenshots

[webteam-app]

Demo

Jenkins

demos.haus

[jmuzina]

This is progressing nicely, but I've noticed that several snapshots are missing. It looks like some error is being thrown for snapshots that are not combined. Error message is::

[percy:core] Error: Mysql2::Error: Duplicate entry '35527075-2565' for key 'index_test_case_executions_on_build_id_and_test_case_id'
    at handleFinished (file:///home/runner/work/vanilla-framework/vanilla-framework/node_modules/@percy/client/dist/utils.js:200:17)
    at IncomingMessage.<anonymous> (file:///home/runner/work/vanilla-framework/vanilla-framework/node_modules/@percy/client/dist/utils.js:216:27)
    at IncomingMessage.emit (node:events:531:35)
    at endReadableNT (node:internal/streams/readable:1696:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) (0ms)

Sent a message to Browserstack for help diagnosing. Blocked until I have received response from them.

[jmuzina]

Still awaiting response from Browserstack, so I separated the test case grouping into its own task. This PR will just implement the simplified breakpoint/theming grouping

[pastelcyborg]

I must be missing some context here. Why are we not capturing screenshots of each theme at every breakpoint?

[jmuzina]

@pastelcyborg

I must be missing some context here. Why are we not capturing screenshots of each theme at every breakpoint?

We currently take one snapshot per theme at desktop size, and one snapshot per additional breakpoint on default theme (with no theme query param specified). So for a given example (say, tooltips combined, you get the following snapshots)

Query Param	Widths
?theme=light	[1280]
?theme=paper	[1280]
?theme=dark	[1280]
N/A (defaults to light)	[375]

This means you have to review four different snapshot groupings... multiplied by two for standalone.

This change will take one snapshot per breakpoint on light theme, and an additional desktop snapshot per color theme. So that would change to:

Query Param	Widths
?theme=light	[375, 1280]
?theme=paper	[1280]
?theme=dark	[1280]

The amount of snapshots (4) is the same, but Percy makes it easier to find the light responsive snapshot by grouping them together like this if you specify multiple widths for the same url:

Rather than having to scroll through and select the other snapshot in the snapshots list, like you currently do:

We will have 6 snapshot groupings per example, rather than 8.

[jmuzina]

Why are we not capturing screenshots of each theme at every breakpoint?

We intentionally don't capture at every breakpoint/theme combination. We assume that snapshot responsive content handling doesn't vary between themes at the same breakpoint, so we only capture the smaller breakpoints on desktop.

[pastelcyborg]

Just to make sure I understand this: we don't currently take mobile screenshots for dark and paper theme? I didn't realize that; that's not optimal. I sort of understand the logic, but I'd probably suggest we take screenshots of all themes across all breakpoints eventually.

That's unrelated from this PR though; thanks for explaining, this sounds good otherwise.

[jmuzina]

Just to make sure I understand this: we don't currently take mobile screenshots for dark and paper theme? I didn't realize that; that's not optimal. I sort of understand the logic, but I'd probably suggest we take screenshots of all themes across all breakpoints eventually.

@pastelcyborg This comes from this MM message from @bartaz - it was done as a way of reducing test volume I believe. If we captured in all themes and breakpoints, we'd expect to see test volume increase by around 3x. However, I do agree that it's optimal to snapshot every breakpoint in every theme for thoroughness' sake.

[bartaz]

Just to make sure I understand this: we don't currently take mobile screenshots for dark and paper theme? I didn't realize that; that's not optimal. I sort of understand the logic, but I'd probably suggest we take screenshots of all themes across all breakpoints eventually.

@pastelcyborg This comes from this MM message from @bartaz - it was done as a way of reducing test volume I believe. If we captured in all themes and breakpoints, we'd expect to see test volume increase by around 3x. However, I do agree that it's optimal to snapshot every breakpoint in every theme for thoroughness' sake.

Now that I've been called out directly it feels I need to justify myself :)

Yes, it felt a bit redundant to create all the snapshots in all themes (3 of them) and all screen sizes (2 or 3 of them), building up to 6 or 9 snapshots per example. Especially that they will all change at the same time in a same way.
In our current implementation on theming the only changes between themes are colours, not layout. So it feels unnecessary to create screenshots of medium and screen sizes in all themes, because they are just the same.

That being said, I agree, that it would be better/safer to screenshot everything. But is it worth the increased cost?

Overall, not part of this PR, we can consider this, but I'm afraid it will bring us back to previous snapshot usage (or even above).

[bartaz]

LGTM