fix: add tests to expand-collapse-content#6628
Conversation
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
| const e = await new Promise(resolve => { | ||
| content.addEventListener('d2l-expand-collapse-content-expand', (e) => resolve(e), { once: true }); | ||
| [true, false].forEach(expand => { | ||
| const event = `d2l-expand-collapse-content-${expand ? 'expand' : 'collapse'}`; |
There was a problem hiding this comment.
Nit: we often code search for event names when trying to find consumers, either when we're making breaking changes or when we're considering removing an event altogether. I'd suggest just using the full event name in the string instead of concatenating it.
| await content.updateComplete; | ||
| await nextFrame(); | ||
| await nextFrame(); |
There was a problem hiding this comment.
This feels pretty fragile. I'd suggest just using await waitUntil(() => content._state === expand ? states.EXPANDED : states.COLLAPSING).
| /* eslint-disable lit/no-private-properties */ | ||
| return html` | ||
| <d2l-expand-collapse-content | ||
| ._reduceMotion="${!this.transitions}" |
There was a problem hiding this comment.
Disabling reduced motion is going to cause the tests to run quite slowly. We don't normally do this for that reason, but I guess it could be worth it if we think testing it is valuable enough.
There was a problem hiding this comment.
Yeah I thought the same, before I made this PR while testing the refactored expand-collapse I was mocking the transitionend event and that was passing and working fine.
But with the current expand-collapse the timing was off and it was because this version solved the no-height defect by forcing the expanded state.
Then I thought about mocking the transition event only when the component was empty, but then it felt very implementation-specific.
With this tests, my refactored version now fails since it has the same defect, so this successfully captured the regression.
|
🎉 This PR is included in version 3.219.8 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
GAUD-9516
As part of inspiration I'm working with copilot to reduce the number of updates requested by our components. While refactoring
d2l-expand-collapse-contentI noticed the tests where only capturing that events were fired and not much else.This PR adds some more robust tests, including:
EXPANDING