Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

US156331 - Add ability to indicate the actual element to use for size calculations for the screenshot #146

Merged
merged 6 commits into from
Aug 25, 2023
Merged

US156331 - Add ability to indicate the actual element to use for size calculations for the screenshot #146

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9f9def3
Support setting vdiff-size to indicate the element with the actual si…
svanherk Aug 24, 2023
da9ac4b
Updating vdiff goldens (#147)
github-actions[bot] Aug 24, 2023
db788a4
Put tests back now that new goldens merged
svanherk Aug 24, 2023
1073c67
Rename to vdiff-rect
svanherk Aug 24, 2023
74a9b7f
Rename to vdiff-target
svanherk Aug 25, 2023
dbcc21e
Merge branch 'main' into US156331_Add_ability_to_find_real_vdiff_size
svanherk Aug 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/browser/vdiff.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ mocha.setup({
});
/* eslint-enable */

function findRealSize(elem) {
if (!elem.shadowRoot) return elem;
const nestedSizedElem = elem.shadowRoot.querySelector('.vdiff-size');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternate options for the CSS class: vdiff-rect-elem, vdiff-rect, vdiff-container

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I had vdiff-rect and then I didn't love it, but I don't love vdiff-size either so switched back to that!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throwing another option out there: vdiff-boundary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vdiff-area? vdiff-box? vdiff-frame? vdiff-actual?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vdiff-target?

if (!nestedSizedElem) return elem;
return findRealSize(nestedSizedElem);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. So if an element doesn't define its size at all but instead that falls to something in its shadow root, it can just put the class on that element and it'll recurse -- nice!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah exactly! And in theory, this should "just work" for consumers wrapping a core component in that situation, because we would've had to already add it for everything that needs it for our own tests.

}

async function ScreenshotAndCompare(opts) {

if (window.d2lTest) {
Expand All @@ -27,7 +34,11 @@ async function ScreenshotAndCompare(opts) {
}

const name = this.test.fullTitle();
const rect = this.elem === document ? null : this.elem.getBoundingClientRect();
let rect = null;
if (this.elem !== document) {
const realSizedElem = findRealSize(this.elem);
rect = realSizedElem.getBoundingClientRect();
}
const slowDuration = this.test.slow();
let result = await executeServerCommand('brightspace-visual-diff-compare', { name, rect, slowDuration, opts });
if (result.resizeRequired) {
Expand Down
1 change: 1 addition & 0 deletions src/server/wtr-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ export class WTRConfig {
config.groups.forEach(g => g.browsers = this.getBrowsers(g.browsers));

if (open || watch) {
config.testsFinishTimeout = 15 * 60 * 1000;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit unrelated, but when I was trying to debug my new tests with --open, I only had two minutes before the whole thing force-closed. This ups that timeout to 15 minutes in open or watch mode.

config.plugins ??= [];
const currentPattern = files || config.groups.find(g => g.name === group)?.files;

Expand Down
49 changes: 47 additions & 2 deletions test/browser/element.vdiff.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { css, html, LitElement } from 'lit';
import { defineCE, expect, fixture, hoverElem } from '../../src/browser/index.js';
import { executeServerCommand } from '@web/test-runner-commands';
//import { executeServerCommand } from '@web/test-runner-commands';
import { unsafeHTML } from 'lit/directives/unsafe-html.js';

const elementTag = defineCE(
class extends LitElement {
Expand Down Expand Up @@ -39,6 +40,40 @@ const elementTag = defineCE(
}
);

const fixedElementTag = defineCE(
class extends LitElement {
static get styles() {
return css`
div {
background-color: white;
border: 1px solid purple;
border-radius: 8px;
box-shadow: 0 0 10px rgba(0, 0, 0, 0.1);
box-sizing: border-box;
position: fixed;
}
`;
}
render() {
return html`
<div class="vdiff-size">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This represents typical core usage. We'll add the appropriate class to dialog, dropdown`, etc.

<slot></slot>
</div>
`;
}
}
);

const nestedElementTag = defineCE(
class extends LitElement {
render() {
return html`${unsafeHTML(`
<${fixedElementTag} class="vdiff-size"><${elementTag} text="Visual Difference"></${elementTag}></${fixedElementTag}>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This represents what is likely to happen for consumers of core. If they have a component that wraps d2l-dialog for example, they'll add the class to d2l-dialog. vdiff will look for this class, then follow it into d2l-dialog to use the element we've added the class to.

`)}`;
}
}
);

describe('element-matches', () => {
[
{ name: 'default' },
Expand All @@ -58,6 +93,16 @@ describe('element-matches', () => {
await fixture(`<${elementTag} text="Visual Difference"></${elementTag}>`, { viewport: { width: 500, height: 500 } });
await expect(document).to.be.golden();
});

it('true size', async() => {
const elem = await fixture(`<${fixedElementTag}><${elementTag} text="Visual Difference"></${elementTag}></${fixedElementTag}>`, { viewport: { width: 500, height: 500 } });
await expect(elem).to.be.golden();
});

it('nested true size', async() => {
const elem = await fixture(`<${nestedElementTag}></${nestedElementTag}>`, { viewport: { width: 500, height: 500 } });
await expect(elem).to.be.golden();
});
});

describe('element-different', () => {
Expand Down Expand Up @@ -87,7 +132,7 @@ describe('element-different', () => {
].forEach(({ name, action }) => {
it(name, async() => {
const elem = await fixture(`<${elementTag} text="Visual Difference"></${elementTag}>`);
const isGolden = await executeServerCommand('vdiff-get-golden-flag');
const isGolden = true;//await executeServerCommand('vdiff-get-golden-flag');
if (!isGolden) {
await action(elem);
await elem.updateComplete;
Expand Down
Loading