-
Notifications
You must be signed in to change notification settings - Fork 191
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
chore: update to lit@3.0 #4292
chore: update to lit@3.0 #4292
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 |
---|---|---|
|
@@ -31,7 +31,6 @@ import { Popover } from '@spectrum-web-components/popover'; | |
import '@spectrum-web-components/theme/sp-theme.js'; | ||
import { sendMouse } from '../../../test/plugins/browser.js'; | ||
import { sendKeys } from '@web/test-runner-commands'; | ||
import { isChrome } from '@spectrum-web-components/shared'; | ||
|
||
export const runOverlayTriggerTests = (type: string): void => { | ||
describe(`Overlay Trigger - ${type}`, () => { | ||
|
@@ -47,6 +46,7 @@ export const runOverlayTriggerTests = (type: string): void => { | |
justify-content: center; | ||
} | ||
</style> | ||
<input type="text" /> | ||
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. is this deliberate? 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. Yes, the test was giving issues getting into "focus visible", and this makes it easier to enforce that by allowing the test to |
||
<overlay-trigger id="trigger" placement="top"> | ||
<sp-button | ||
id="outer-button" | ||
|
@@ -106,12 +106,6 @@ export const runOverlayTriggerTests = (type: string): void => { | |
</div> | ||
` | ||
); | ||
await nextFrame(); | ||
await nextFrame(); | ||
await nextFrame(); | ||
await nextFrame(); | ||
await nextFrame(); | ||
await nextFrame(); | ||
|
||
this.innerTrigger = this.testDiv.querySelector( | ||
'#inner-trigger' | ||
|
@@ -134,6 +128,16 @@ export const runOverlayTriggerTests = (type: string): void => { | |
this.hoverContent = this.testDiv.querySelector( | ||
'#hover-content' | ||
) as HTMLDivElement; | ||
|
||
await Promise.all([ | ||
this.innerTrigger.updateComplete, | ||
this.outerTrigger.updateComplete, | ||
this.innerButton.updateComplete, | ||
this.outerButton.updateComplete, | ||
this.innerClickContent.updateComplete, | ||
this.outerClickContent.updateComplete, | ||
]); | ||
this.testDiv.querySelector('input').focus(); | ||
}); | ||
|
||
it('opens a popover', async function () { | ||
|
@@ -451,13 +455,6 @@ export const runOverlayTriggerTests = (type: string): void => { | |
}); | ||
|
||
it('escape closes an open popover', async function () { | ||
if (isChrome()) { | ||
// Does a werid test time bleed that allows the `Escape` press through to the | ||
// parent modal. Manual testing does not exhibit this interaction, which seems | ||
// to step from how long the `Escape` button in down. | ||
this.skip(); | ||
} | ||
|
||
this.outerTrigger.type = 'modal'; | ||
this.innerTrigger.type = 'modal'; | ||
const outerOpen = oneEvent(this.outerButton, 'sp-opened'); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,7 @@ | |
}, | ||
"devDependencies": { | ||
"@open-wc/building-rollup": "^3.0.2", | ||
"@open-wc/eslint-config": "^12.0.0", | ||
"@open-wc/eslint-config": "^12.0.3", | ||
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. is this updated required in this PR? |
||
"@rollup/plugin-commonjs": "^25.0.7", | ||
"@types/node": "^20.11.11", | ||
"@typescript-eslint/eslint-plugin": "^6.15.0", | ||
|
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.
if we already trying to find 'open' attributes from the PropertyValues do we also need a check on
this.open
?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.
This specific says, when
open
has changed, but NOT when open has changed fromundefined
(its initial value) tofalse
(its default value). That was the if only returns true when it's not the first update or the end user appliedopen === true
in the DOM of the element before it was upgraded.