-
Notifications
You must be signed in to change notification settings - Fork 25
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.
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
switch to @brightspace-ui/testing fixture #3764
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import '../dialog.js'; | ||
import { expect, fixture, oneEvent } from '@open-wc/testing'; | ||
import { expect, fixture, oneEvent } from '@brightspace-ui/testing'; | ||
import { getComposedActiveElement } from '../../../helpers/focus.js'; | ||
import { html } from 'lit'; | ||
import { runConstructor } from '../../../tools/constructor-test-helper.js'; | ||
|
@@ -17,7 +17,8 @@ describe('d2l-dialog', () => { | |
describe('focus management', () => { | ||
|
||
it('should focus on close button if no focusable elements inside', async() => { | ||
const el = await fixture(html`<d2l-dialog opened>not focusable</d2l-dialog>`); | ||
const el = await fixture(html`<d2l-dialog>not focusable</d2l-dialog>`); | ||
el.opened = true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As discussed previously, these types of tests where an event is fired as part of boot-up need to be adjusted to control when the event gets fired themselves. |
||
await oneEvent(el, 'd2l-dialog-open'); | ||
expect(getComposedActiveElement().getAttribute('aria-label')).to.equal('Close this dialog'); | ||
}); | ||
|
@@ -52,16 +53,18 @@ describe('d2l-dialog', () => { | |
}); | ||
|
||
it('should not focus on an autofocus element that had not been made focusable', async() => { | ||
const el = await fixture(html`<d2l-dialog opened><p autofocus>focus</p><button>focus</button></d2l-dialog>`); | ||
const paragraph = el.querySelector('p'); | ||
const el = await fixture(html`<d2l-dialog><p autofocus>focus</p><button>focus</button></d2l-dialog>`); | ||
el.opened = true; | ||
await oneEvent(el, 'd2l-dialog-open'); | ||
const paragraph = el.querySelector('p'); | ||
expect(getComposedActiveElement()).to.not.equal(paragraph); | ||
}); | ||
|
||
it('should not focus on a descendant autofocus element that had not been made focusable', async() => { | ||
const el = await fixture(html`<d2l-dialog opened><div><p autofocus>focus</p></div><button>focus</button></d2l-dialog>`); | ||
const paragraph = el.querySelector('p'); | ||
const el = await fixture(html`<d2l-dialog><div><p autofocus>focus</p></div><button>focus</button></d2l-dialog>`); | ||
el.opened = true; | ||
await oneEvent(el, 'd2l-dialog-open'); | ||
const paragraph = el.querySelector('p'); | ||
expect(getComposedActiveElement()).to.not.equal(paragraph); | ||
}); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
import '../dropdown.js'; | ||
import '../dropdown-content.js'; | ||
import { aTimeout, expect, fixture, html, nextFrame, oneEvent } from '@open-wc/testing'; | ||
import { focusElem } from '@brightspace-ui/testing'; | ||
import { aTimeout, expect, fixture, focusElem, html, nextFrame, oneEvent } from '@brightspace-ui/testing'; | ||
import { runConstructor } from '../../../tools/constructor-test-helper.js'; | ||
|
||
const normalFixture = html` | ||
|
@@ -62,7 +61,6 @@ describe('d2l-dropdown', () => { | |
beforeEach(async() => { | ||
dropdown = await fixture(normalFixture); | ||
content = dropdown.querySelector('d2l-dropdown-content'); | ||
await content.updateComplete; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do more of this kind of thing in a follow-up PR. |
||
}); | ||
|
||
describe('constructor', () => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've re-run this many times locally and in CI.
It's admittedly not great, but also not shocking since we're now
await
-ing all nested Lit components for ~1650 tests, and doing a lot more resetting of the browser state between tests. If the slowness becomes a problem, there's a lot of tests using timeouts that we could look into fixing up.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think it's worth doing some cleanup to get a better sense of the real timing