Spike into visual-diffs #3610

dlockhart · 2023-05-05T14:54:09Z

This isn't intended to ever merge, but goes alongside this spike results document to show how things could look in practice.

Ultimately, a lot of this will get broken out into a new @brightspace-ui/testing-helpers library.

dlockhart · 2023-05-05T14:54:39Z

components/button/test/button-icon.vdiff.js

@@ -0,0 +1,33 @@
+import '../button-icon.js';
+import { fixture, focusWithKeyboard, focusWithMouse, hoverWithMouse, html, screenshotAndCompare } from '../../../tools/web-test-runner-helpers.js';


Imagine these being imported from @brightspace-ui/testing-helpers.

dlockhart · 2023-05-05T14:55:36Z

components/icons/icon.js

@@ -6,8 +6,9 @@ import { loadSvg } from '../../generated/icons/presetIconLoader.js';
 import { RtlMixin } from '../../mixins/rtl/rtl-mixin.js';
 import { runAsync } from '../../directives/run-async/run-async.js';
 import { unsafeSVG } from 'lit/directives/unsafe-svg.js';
+import { WaitForMeMixin } from '../../tools/web-test-runner-helpers.js';


Example of a component that does asynchronous work (dynamically importing the icon) that we want to wait for before visual-diffing.

dlockhart · 2023-05-05T14:56:25Z

tools/visual-diff-plugin.js

This is the @web/test-runner plugin that consumers don't need to know about and will likely be buried in @brightspace-ui/testing-helpers.

dlockhart · 2023-05-05T14:56:55Z

tools/visual-diff-reporter.js

This doesn't do anything really, but is an example of a custom reporter. This is where we'd implement the generation of our HTML report files.

dlockhart · 2023-05-05T14:57:15Z

tools/web-test-runner-helpers.js

Most if not all of this will live in @brightspace-ui/testing-helpers.

dlockhart · 2023-05-05T14:57:29Z

tools/web-test-runner-helpers.js

+	return await document.fonts.ready;
+}
+
+export const WaitForMeMixin = dedupeMixin(superclass => class extends superclass {


This will likely be defined in @brightspace-ui/core.

dlockhart · 2023-05-05T14:58:20Z

web-test-runner.config.js

@@ -20,8 +38,47 @@ export default {
 					}
 				})
 			]
+		},


I think we'll want to take all of this and turn it into a shared config that consumers can just import from @brightspcae-ui/testing-helpers.

svanherk · 2023-05-09T14:24:42Z

components/button/test/button-icon.vdiff.js

+		{ category: 'normal', f: html`<d2l-button-icon icon="tier1:gear" text="Icon Button"></d2l-button-icon>` },
+		{ category: 'translucent', f: html`<d2l-button-icon icon="tier1:gear" text="Icon Button" translucent></d2l-button-icon>`, theme: 'translucent' },
+		{ category: 'dark', f: html`<d2l-button-icon icon="tier1:gear" text="Icon Button" theme="dark"></d2l-button-icon>`, theme: 'dark' },
+		{ category: 'custom', f: html`<d2l-button-icon icon="tier1:gear" text="Icon Button" style="--d2l-button-icon-min-height: 1.5rem; --d2l-button-icon-min-width: 1.5rem; --d2l-button-icon-border-radius: 4px; --d2l-button-icon-focus-box-shadow: 0 0 0 1px #ffffff, 0 0 0 3px #006fbf; --d2l-button-icon-fill-color: var(--d2l-color-celestine); --d2l-button-icon-fill-color-hover: var(--d2l-color-celestine-minus-1);"></d2l-button-icon>` }


Would it be possible to pass in a css template literal to fixture that would automatically get added and removed from the page? 🤔 Could clean this kind of thing up a bit

Maybe? We have to be careful because we still want the root of the fixture to be the button, so we couldn't drop a <style> element in as a sibling. But maybe we could do a constructible stylesheet and apply it to the <body>. Worth experimenting for sure, I'll add it to the Confluence doc.

svanherk · 2023-05-15T14:39:43Z

Rerunning the jobs here to test the new configure-aws-credentials version

svanherk · 2023-06-13T16:01:11Z

tools/web-test-runner-helpers.js

+
+export const screenshotAndCompare = async(elem, name, opts) => {
+
+	const rect = elem.getBoundingClientRect();


We currently do some fancy logic to handle getting the proper rect for dropdowns (and filter, etc.): https://github.com/BrightspaceUI/core/blob/main/components/dropdown/test/dropdown-helper.js#L28-L44
Do we see that being added here, or providing users a way to override? Or should they create a "large enough" wrapper element?

Making a wrapper element that is "large enough" is the simplest, although adjusting the hard-coded dimensions any time the contents change is a bit of an annoyance. It can provide some degree of buffer when contents change that would normally leave us without a diff due to the size changing, though my understanding is that we're going to improve that aspect. I tend to like being able to override with something more automatic like this.

Having said that, the consumer could always have some script that resizes the wrapper before the screenshot.

What if they could do something like this in fixture:

const elem = await fixture(html`<d2l-my-elem>...</d2l-my-elem>`, { margin: { blockEnd: 200, inlineStart: 50, inlineEnd: 75 } });

Then we'd reserve 200px at the bottom, 50px to the left and 75px to the right in the form of padding on the body (we default to 30px today). Alternately we could just put a huge amount of padding on the body -- like 500px -- but then the viewport size would need to change.

So that would reserve the space initially as it renders. Then when they take the screenshot maybe it can reuse that value when it captures the rectangle? screenshotAndCompare already supports a single-value "margin" option (defaults to 10px all around).

I think that would work if the consumer hard-codes it.

Seems like that wouldn't work if the consumer wants to avoid hard-coding - if they wanted to measure the opener + the content to figure out the size needed (like the helper Stacey linked), it needs to have access to the element, but it won't exist yet. 🐔 & 🥚 ... 😄 . It would need a way to adjust margin after it's been rendered. Or, for the rare cases where a consumer might want this, they just include a wrapper div in their fixture and they set the dimensions of it based on measurement just before doing the screenshot.

github-actions · 2023-08-25T14:28:59Z

Thanks for the PR! 🎉

We've deployed an automatic preview for this PR - you can see your changes here:

URL	https://live.d2l.dev/prs/BrightspaceUI/core/pr-3610/

Note
The build needs to finish before your changes are deployed.
Changes to the PR will automatically update the instance.

svanherk · 2023-10-06T19:16:47Z

@dlockhart Should be good to close now!

dlockhart commented May 5, 2023

View reviewed changes

svanherk reviewed May 9, 2023

View reviewed changes

bearfriend force-pushed the dlockhart/visual-diff-spike branch from 8a4dbc1 to e382877 Compare June 2, 2023 18:24

bearfriend mentioned this pull request Jun 12, 2023

Move visual-diff-plugin to testing repo BrightspaceUI/testing#16

Merged

svanherk reviewed Jun 13, 2023

View reviewed changes

dlockhart force-pushed the dlockhart/visual-diff-spike branch from ad3e74b to f50ecf0 Compare June 28, 2023 18:34

bearfriend force-pushed the dlockhart/visual-diff-spike branch 3 times, most recently from 549294c to c3feed2 Compare July 19, 2023 18:53

dlockhart and others added 6 commits July 31, 2023 13:24

spike into visual-diffs

77249f2

update to use @brightspace-ui/testing vdiff

105d28d

Fix things

25e335e

Fix flaky vdiff tests

2acbf8e

Fix firefox vdiff focus by forwarding focus-visible

b14ae64

Fix firefox flaky vdiff tests

49d7790

bearfriend force-pushed the dlockhart/visual-diff-spike branch from c3feed2 to 49d7790 Compare July 31, 2023 17:26

BrightspaceUI deleted a comment from github-actions bot Aug 25, 2023

Merge branch 'main' into dlockhart/visual-diff-spike

2e0befe

github-actions bot mentioned this pull request Aug 25, 2023

Updating vdiff goldens for PR 3610 #3923

Closed

bearfriend mentioned this pull request Aug 30, 2023

Migrate button vdiff tests #3949

Merged

dlockhart closed this Oct 6, 2023

dlockhart deleted the dlockhart/visual-diff-spike branch October 6, 2023 19:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spike into visual-diffs #3610

Spike into visual-diffs #3610

dlockhart commented May 5, 2023

dlockhart May 5, 2023

dlockhart May 5, 2023

dlockhart May 5, 2023

dlockhart May 5, 2023

dlockhart May 5, 2023

dlockhart May 5, 2023

dlockhart May 5, 2023

svanherk May 9, 2023

dlockhart May 9, 2023

svanherk commented May 15, 2023

svanherk Jun 13, 2023

dbatiste Jun 13, 2023

dbatiste Jun 13, 2023

dlockhart Jun 13, 2023

dbatiste Jun 13, 2023

github-actions bot commented Aug 25, 2023

svanherk commented Oct 6, 2023

		@@ -0,0 +1,33 @@
		import '../button-icon.js';
		import { fixture, focusWithKeyboard, focusWithMouse, hoverWithMouse, html, screenshotAndCompare } from '../../../tools/web-test-runner-helpers.js';

@@ @@ -20,8 +38,47 @@ export default { @@
               					}
               				})
               			]
+              		},


		export const screenshotAndCompare = async(elem, name, opts) => {

		const rect = elem.getBoundingClientRect();

Spike into visual-diffs #3610

Spike into visual-diffs #3610

Conversation

dlockhart commented May 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svanherk commented May 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 25, 2023

svanherk commented Oct 6, 2023