🐛 [bento][amp-accordion] Update accordion to dispatch event with correct name #32141
Conversation
| // Set up section 1 to trigger expand of section 3 on expand | ||
| // and collapse of section 3 on collapse | ||
| const api = await element.getApi(); | ||
| section1.addEventListener('expand', () => api.expand('section3')); |
There was a problem hiding this comment.
Probably would be nice to just have a test where we check that the event has been delivered.
E.g.
const spy = sandbox.spy();
section1.addEventListener('expand', spy);
// do work
expect(spy).to.be.calledOnce.
There was a problem hiding this comment.
I added spies to this unit test. Do you think it's too convoluted this way?
There was a problem hiding this comment.
Correct me if I'm wrong, but I interpreted the suggestion as to use spies instead of propagating events to an additional section. Or if doing both, separate them as distinct it blocks (one which checks spies, one which copies mimics events onto the other section).
There was a problem hiding this comment.
Sure, no problem. I've updated the tests to two separate tests.
|
Adding @caroqliu as FYI. Didn't add you at first since the fix seemed trivial. But the tests might be interesting to you. |
| // Set up section 1 to trigger expand of section 3 on expand | ||
| // and collapse of section 3 on collapse | ||
| const api = await element.getApi(); | ||
| section1.addEventListener('expand', () => api.expand('section3')); |
There was a problem hiding this comment.
Correct me if I'm wrong, but I interpreted the suggestion as to use spies instead of propagating events to an additional section. Or if doing both, separate them as distinct it blocks (one which checks spies, one which copies mimics events onto the other section).
…ect name (ampproject#32141) * Fix a bug in bento accordion * Add spy functions * Separate tests for listening to events
Fix a bug in amp-accordion. amp-accordion was dispatching events with the incorrect name causing a bug when attempting to listen for events in bento mode.