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.

Sign up for GitHub

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

Jump to bottom

feat(webchat): allow users to subscribe to different webchat events #522

Merged
merged 9 commits into from
Aug 26, 2022
Merged

feat(webchat): allow users to subscribe to different webchat events #522

merged 9 commits into from
Aug 26, 2022

Conversation

EFF
Copy link
Member

@EFF EFF commented Aug 24, 2022

Done:

  • converted inject.js to typescript ==> note that I didn't change the code much, I didn't want to go for a full re-implementation
  • implemented event filtering and handling logic in the already existing window message listener in inject.ts ==> this not only improves the user experience it also prevent the user to create multiple window event listeners
  • converted event types to types that are properly categorized
  • fixed some typings along the way

@EFF EFF requested a review from laurentlp August 24, 2022 12:59
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 24, 2022
View reviewed changes
packages/webchat/src/utils/webchatEvents.ts

export const postMessageToParent = (type: WebchatEventType, value: any, chatId: string) => {
const evt: WebchatEvent = { type, value, chatId }
window.parent?.postMessage(evt, '*')

Check warning

Code scanning / CodeQL

Cross-window communication with unrestricted target origin

Sensitive data returned from [here](1) is sent to another window without origin restriction. Sensitive data returned from [here](2) is sent to another window without origin restriction. Sensitive data returned from [here](3) is sent to another window without origin restriction.
laurentlp
laurentlp reviewed Aug 24, 2022
View reviewed changes
package.json Show resolved Hide resolved
packages/inject/src/inject.ts Outdated Show resolved Hide resolved
packages/inject/src/inject.ts Outdated Show resolved Hide resolved
packages/inject/src/inject.ts Outdated Show resolved Hide resolved
packages/inject/src/inject.ts Outdated Show resolved Hide resolved
packages/inject/src/inject.ts Show resolved Hide resolved
packages/inject/src/inject.ts Outdated Show resolved Hide resolved
packages/inject/src/inject.ts Outdated Show resolved Hide resolved
packages/webchat/src/main.tsx Show resolved Hide resolved
packages/webchat/src/main.tsx Show resolved Hide resolved
@EFF
fix(webchat): properly serialize message (#523)
91b997f 
* fix(webchat): properly serialize message

* reference to iframe window instead or postmessage

* set selected message when message is received

* pull request comments

* more comment fix
@EFF EFF merged commit d41eb38 into master Aug 26, 2022
@EFF EFF deleted the f_webchat-post-new-messages branch August 26, 2022 14:30
@laurentlp
Copy link
Contributor

Note: The webchat test on firefox never completes on the CI but was tested locally and ran with success!

@botpress-bot botpress-bot bot mentioned this pull request Sep 23, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurentlp laurentlp laurentlp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@EFF @laurentlp