Skip to content
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

bind this to message event listener #111

Conversation

kevinwhereby
Copy link
Contributor

@kevinwhereby kevinwhereby commented Oct 31, 2023

this in onmessage was bound to the parent context, so events were never
being forwarded from the iframe, as the this.url.origin was always
undefined.

Removes the need for the null coalesce in this.url?.origin as there
will be no events triggered before this property is set in render

to test

  1. add current beta1 to some test project, render the iframe element
  2. document.querySelector("whereby-embed").addEventListener("camera-toggle", () => console.log("Triggered"))
  3. toggle camera a few times and see there are no messages
  4. replace current beta1 with this branch
  5. step 2 & 3
  6. see the messages are received

gotchas

I tried adding a test for this (since we didn't detect this regression when it happened), but the PWA requirement to accept cam/mic permissions means it won't work in CI

@kevinwhereby kevinwhereby force-pushed the kevinhanna/pan-359-iframe-events-dont-work-in-the-alpha-build branch 2 times, most recently from da02a77 to 2e705b1 Compare October 31, 2023 12:31
`this` in onmessage was bound to the parent window, so events were never
being forwarded from the iframe, as the `this.url.origin` was always
undefined.

Removes the need for the null coalesce in `this.url?.origin` as there
will be no events triggered before this property is set in `render`
@kevinwhereby kevinwhereby force-pushed the kevinhanna/pan-359-iframe-events-dont-work-in-the-alpha-build branch from 2e705b1 to b28fadd Compare October 31, 2023 12:31
@kevinwhereby kevinwhereby requested a review from a team October 31, 2023 12:52
@kevinwhereby kevinwhereby merged commit 4c8b6ff into development Oct 31, 2023
2 checks passed
@kevinwhereby kevinwhereby deleted the kevinhanna/pan-359-iframe-events-dont-work-in-the-alpha-build branch October 31, 2023 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants