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

Merge production Web Chat #623

Merged
merged 7 commits into from
May 31, 2018
Merged

Conversation

compulim
Copy link
Contributor

@compulim compulim commented May 31, 2018

Now, we can use production Web Chat without modifications in Emulator:

  • No custom build of Web Chat and Direct Line JS, we are using production version from NPM directly
  • Chat history preservation/restoration is done thru Emulator Core, not hacking into Web Chat redux store
  • Chat tabs behind the current tab will no longer unmount, but hidden and kept in DOM until close

Caveats:

After merging this PR, we will show send box and interactivity even we load transcript, this is because the PR for hiding send box is pending approval on Web Chat. Once it's in Web Chat, we can bump version and get that fixed without code change. Will track in another bug.

Some design considerations:

  • Bot ID and user ID are now fixed to "bot" and "default-user" respectively
    • Otherwise, we will need to load physical transcript files before instantiating the document editor (need context switching: client -> main -> client)
    • Web Chat is not reactive to bot ID and user ID change
    • No one complain about random bot ID and user ID for Chatdown-generated transcript, seems no one really care about bot ID and user ID yet
  • Tabs that hide behind active tab are now hidden, instead of unmounted
    • Otherwise, we will need to preserve Web Chat redux store, which means
      1. Preserving history in Web Chat is controversy and we probably need to keep custom Web Chat, instead of production
      2. Emulator redux store will need to keep a copy of another redux store (as of today), seems weird, and Web Chat redux store is not plain JSON too

@@ -148,7 +147,7 @@ export function newDocument(documentId: string, mode: ChatMode, additionalData?:
mode,
documentId,
conversationId: null,
webChatStore: createWebChatStore(),
// webChatStore: createWebChatStore(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this no longer required? If so can we just delete the line?

@emadelwany
Copy link
Contributor

@compulim so good to see so much custom code get deleted! Awesome PR 👍

Copy link
Contributor

@justinwilaby justinwilaby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. :shipit:

@compulim compulim merged commit f78d474 into microsoft:v4 May 31, 2018
@compulim compulim deleted the feat-merge-webchat-rebased branch May 31, 2018 21:46
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.

None yet

3 participants