From c5e4dd8106cf08a73a5e27f2465b8fe4f53379a2 Mon Sep 17 00:00:00 2001 From: Jeremias Peier Date: Thu, 12 Sep 2024 10:29:46 +0200 Subject: [PATCH 1/5] test: respect state element for visual focus tests --- .../private/visual-regression-snapshot.ts | 4 +- .../datepicker-next-day.visual.spec.ts | 73 +++++++------------ .../datepicker-previous-day.visual.spec.ts | 5 ++ .../form-field/form-field.visual.spec.ts | 11 +-- 4 files changed, 38 insertions(+), 55 deletions(-) diff --git a/src/elements/core/testing/private/visual-regression-snapshot.ts b/src/elements/core/testing/private/visual-regression-snapshot.ts index cfe6c02e1e..2d00bf132d 100644 --- a/src/elements/core/testing/private/visual-regression-snapshot.ts +++ b/src/elements/core/testing/private/visual-regression-snapshot.ts @@ -113,7 +113,9 @@ export const visualDiffFocus: VisualDiffState = { with(setup: (setup: VisualDiffSetupBuilder) => void | Promise): Mocha.Func { return async function (this: Mocha.Context) { const builder = await runSetupWithViewport(setup, this.test?.ctx?.['requestViewport']); - builder.snapshotElement.focus(); + builder.stateElement.focus(); + // We focus the state element and then tab left and right again to activate the focus visible state on the desired element + await sendKeys({ press: `Shift+${tabKey}` }); await sendKeys({ press: tabKey }); await visualDiff(builder.snapshotElement, imageName(this.test!)); }; diff --git a/src/elements/datepicker/datepicker-next-day/datepicker-next-day.visual.spec.ts b/src/elements/datepicker/datepicker-next-day/datepicker-next-day.visual.spec.ts index ed279c4dd7..f91910b88e 100644 --- a/src/elements/datepicker/datepicker-next-day/datepicker-next-day.visual.spec.ts +++ b/src/elements/datepicker/datepicker-next-day/datepicker-next-day.visual.spec.ts @@ -1,12 +1,10 @@ -import { sendKeys } from '@web/test-runner-commands'; import { html, nothing } from 'lit'; -import { tabKey } from '../../core/testing/private/keys.js'; import { describeEach, describeViewports, visualDiffDefault, - visualRegressionFixture, + visualDiffFocus, } from '../../core/testing/private.js'; import './datepicker-next-day.js'; @@ -20,51 +18,36 @@ describe(`sbb-datepicker-next-day`, () => { }; describeViewports({ viewports: ['zero', 'medium'] }, () => { - it( - `standalone`, - visualDiffDefault.with(async (setup) => { - await setup.withFixture(html``); - }), - ); - - describeEach(cases, ({ negative, value }) => { - let root: HTMLElement; - - beforeEach(async () => { - root = await visualRegressionFixture( - html` - - - - - - `, - { backgroundColor: negative ? 'var(--sbb-color-black)' : undefined }, - ); - }); - + for (const state of [visualDiffDefault, visualDiffFocus]) { it( - `with form-field`, - visualDiffDefault.with(async (setup) => { - setup.withSnapshotElement(root); + `standalone ${state.name}`, + state.with(async (setup) => { + await setup.withFixture(html``); }), ); - it( - `with form-field focus`, - visualDiffDefault.with(async (setup) => { - setup.withSnapshotElement(root); - - if (value) { - // Focus input so that with a tab press it should land on next day - setup.snapshotElement.querySelector('input')!.focus(); - } else { - setup.snapshotElement.focus(); - } - - await sendKeys({ press: tabKey }); - }), - ); - }); + describeEach(cases, ({ negative, value }) => { + it( + `with form-field ${state.name}`, + state.with(async (setup) => { + await setup.withFixture( + html` + + + + + + `, + { backgroundColor: negative ? 'var(--sbb-color-black)' : undefined }, + ); + setup.withStateElement( + value + ? setup.snapshotElement.querySelector('sbb-datepicker-next-day')! + : setup.snapshotElement, + ); + }), + ); + }); + } }); }); diff --git a/src/elements/datepicker/datepicker-previous-day/datepicker-previous-day.visual.spec.ts b/src/elements/datepicker/datepicker-previous-day/datepicker-previous-day.visual.spec.ts index 90b2654d16..25cc98b2bb 100644 --- a/src/elements/datepicker/datepicker-previous-day/datepicker-previous-day.visual.spec.ts +++ b/src/elements/datepicker/datepicker-previous-day/datepicker-previous-day.visual.spec.ts @@ -42,6 +42,11 @@ describe(`sbb-datepicker-previous-day`, () => { `, { backgroundColor: negative ? 'var(--sbb-color-black)' : undefined }, ); + setup.withStateElement( + value + ? setup.snapshotElement.querySelector('sbb-datepicker-previous-day')! + : setup.snapshotElement, + ); }), ); }); diff --git a/src/elements/form-field/form-field/form-field.visual.spec.ts b/src/elements/form-field/form-field/form-field.visual.spec.ts index 4a16dddfcd..1148573362 100644 --- a/src/elements/form-field/form-field/form-field.visual.spec.ts +++ b/src/elements/form-field/form-field/form-field.visual.spec.ts @@ -1,8 +1,5 @@ -import { sendKeys } from '@web/test-runner-commands'; import { html, nothing, type TemplateResult } from 'lit'; -import type { SbbMiniButtonElement } from '../../button/mini-button.js'; -import { tabKey } from '../../core/testing/private/keys.js'; import { describeEach, describeViewports, @@ -221,7 +218,7 @@ describe(`sbb-form-field`, () => { it( `slot=buttons focus`, - visualDiffDefault.with(async (setup) => { + visualDiffFocus.with(async (setup) => { const templateResult: TemplateResult = html`${template(args)} ${buttonsAndPopover(args)}`; await setup.withFixture(html`${formField(args, templateResult)}`, { @@ -229,11 +226,7 @@ describe(`sbb-form-field`, () => { focusOutlineDark: negative, forcedColors, }); - ( - setup.snapshotElement.querySelector(name)! - .nextElementSibling as SbbMiniButtonElement - ).focus(); - await sendKeys({ press: tabKey }); + setup.withStateElement(setup.snapshotElement.querySelector('sbb-mini-button')!); }), ); }); From a020ec0adcfc28a298561d98ab822ff0b37fe86b Mon Sep 17 00:00:00 2001 From: Jeremias Peier Date: Thu, 12 Sep 2024 11:04:56 +0200 Subject: [PATCH 2/5] fix: fix logic --- .../core/testing/private/visual-regression-snapshot.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/elements/core/testing/private/visual-regression-snapshot.ts b/src/elements/core/testing/private/visual-regression-snapshot.ts index 2d00bf132d..84a73ee210 100644 --- a/src/elements/core/testing/private/visual-regression-snapshot.ts +++ b/src/elements/core/testing/private/visual-regression-snapshot.ts @@ -114,8 +114,12 @@ export const visualDiffFocus: VisualDiffState = { return async function (this: Mocha.Context) { const builder = await runSetupWithViewport(setup, this.test?.ctx?.['requestViewport']); builder.stateElement.focus(); - // We focus the state element and then tab left and right again to activate the focus visible state on the desired element - await sendKeys({ press: `Shift+${tabKey}` }); + + if (builder.stateElement !== builder.snapshotElement) { + // We focus one element before the state element by tabbing left. The next tab will then focus the state element. + await sendKeys({ press: `Shift+${tabKey}` }); + } + await sendKeys({ press: tabKey }); await visualDiff(builder.snapshotElement, imageName(this.test!)); }; From 00f6a0348308c5f833d90d538badbf9c68a1d382 Mon Sep 17 00:00:00 2001 From: Jeremias Peier Date: Thu, 12 Sep 2024 14:06:17 +0200 Subject: [PATCH 3/5] test: next try --- .../testing/private/visual-regression-snapshot.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/elements/core/testing/private/visual-regression-snapshot.ts b/src/elements/core/testing/private/visual-regression-snapshot.ts index 84a73ee210..ca69333109 100644 --- a/src/elements/core/testing/private/visual-regression-snapshot.ts +++ b/src/elements/core/testing/private/visual-regression-snapshot.ts @@ -113,14 +113,17 @@ export const visualDiffFocus: VisualDiffState = { with(setup: (setup: VisualDiffSetupBuilder) => void | Promise): Mocha.Func { return async function (this: Mocha.Context) { const builder = await runSetupWithViewport(setup, this.test?.ctx?.['requestViewport']); - builder.stateElement.focus(); - - if (builder.stateElement !== builder.snapshotElement) { - // We focus one element before the state element by tabbing left. The next tab will then focus the state element. - await sendKeys({ press: `Shift+${tabKey}` }); - } + // We create a focusable element () right before the state element. + // We can tab once to land on the desired element. + const link = document.createElement(`a`); + link.href = '#'; + link.classList.add('sbb-screen-reader-only'); + builder.stateElement.insertAdjacentElement('beforebegin', link); + link.focus(); await sendKeys({ press: tabKey }); + link.remove(); + await visualDiff(builder.snapshotElement, imageName(this.test!)); }; }, From 274d4e64f4f06eb09b025b9979f21df839f5c525 Mon Sep 17 00:00:00 2001 From: Jeremias Peier Date: Thu, 12 Sep 2024 15:19:06 +0200 Subject: [PATCH 4/5] fix: review --- .../testing/private/visual-regression-snapshot.ts | 2 ++ .../datepicker-next-day.visual.spec.ts | 11 ++++++----- .../datepicker-previous-day.visual.spec.ts | 11 ++++++----- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/elements/core/testing/private/visual-regression-snapshot.ts b/src/elements/core/testing/private/visual-regression-snapshot.ts index ca69333109..53a7f274e7 100644 --- a/src/elements/core/testing/private/visual-regression-snapshot.ts +++ b/src/elements/core/testing/private/visual-regression-snapshot.ts @@ -119,6 +119,8 @@ export const visualDiffFocus: VisualDiffState = { const link = document.createElement(`a`); link.href = '#'; link.classList.add('sbb-screen-reader-only'); + // We need to copy the slot so we can ensure it's landing at the right position + link.slot = builder.stateElement.slot; builder.stateElement.insertAdjacentElement('beforebegin', link); link.focus(); await sendKeys({ press: tabKey }); diff --git a/src/elements/datepicker/datepicker-next-day/datepicker-next-day.visual.spec.ts b/src/elements/datepicker/datepicker-next-day/datepicker-next-day.visual.spec.ts index f91910b88e..3601a3901f 100644 --- a/src/elements/datepicker/datepicker-next-day/datepicker-next-day.visual.spec.ts +++ b/src/elements/datepicker/datepicker-next-day/datepicker-next-day.visual.spec.ts @@ -40,11 +40,12 @@ describe(`sbb-datepicker-next-day`, () => { `, { backgroundColor: negative ? 'var(--sbb-color-black)' : undefined }, ); - setup.withStateElement( - value - ? setup.snapshotElement.querySelector('sbb-datepicker-next-day')! - : setup.snapshotElement, - ); + + if (value) { + setup.withStateElement( + setup.snapshotElement.querySelector('sbb-datepicker-next-day')!, + ); + } }), ); }); diff --git a/src/elements/datepicker/datepicker-previous-day/datepicker-previous-day.visual.spec.ts b/src/elements/datepicker/datepicker-previous-day/datepicker-previous-day.visual.spec.ts index 25cc98b2bb..22bcf849db 100644 --- a/src/elements/datepicker/datepicker-previous-day/datepicker-previous-day.visual.spec.ts +++ b/src/elements/datepicker/datepicker-previous-day/datepicker-previous-day.visual.spec.ts @@ -42,11 +42,12 @@ describe(`sbb-datepicker-previous-day`, () => { `, { backgroundColor: negative ? 'var(--sbb-color-black)' : undefined }, ); - setup.withStateElement( - value - ? setup.snapshotElement.querySelector('sbb-datepicker-previous-day')! - : setup.snapshotElement, - ); + + if (value) { + setup.withStateElement( + setup.snapshotElement.querySelector('sbb-datepicker-previous-day')!, + ); + } }), ); }); From 55c705992313cd471915db9b6a2d14ff8ab81577 Mon Sep 17 00:00:00 2001 From: Jeremias Peier Date: Thu, 12 Sep 2024 18:34:35 +0200 Subject: [PATCH 5/5] fix: rebase --- .../header/header/header.visual.spec.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/elements/header/header/header.visual.spec.ts b/src/elements/header/header/header.visual.spec.ts index b2ce3c05fa..1be81e1a07 100644 --- a/src/elements/header/header/header.visual.spec.ts +++ b/src/elements/header/header/header.visual.spec.ts @@ -1,8 +1,10 @@ -import { sendKeys } from '@web/test-runner-commands'; import { html, type TemplateResult } from 'lit'; -import { tabKey } from '../../core/testing/private/keys.js'; -import { describeViewports, visualDiffDefault } from '../../core/testing/private.js'; +import { + describeViewports, + visualDiffDefault, + visualDiffFocus, +} from '../../core/testing/private.js'; import './header.js'; import '../header-link.js'; @@ -64,14 +66,13 @@ describe(`sbb-header`, () => { describe(logoOrSignet, () => { it( 'focus', - visualDiffDefault.with(async (setup) => { + visualDiffFocus.with(async (setup) => { await setup.withFixture(template(false, logoOrSignet === 'signet' ? 's' : 'm'), { padding: '0', }); - setup.snapshotElement.querySelector(`a[slot='logo']`)!.focus(); - // We focus the logo and then tab left and right again to activate the focus visible state on the link - await sendKeys({ press: `Shift+${tabKey}` }); - await sendKeys({ press: tabKey }); + setup.withStateElement( + setup.snapshotElement.querySelector(`a[slot='logo']`)!, + ); }), ); });