Add ability to customize Web Chat by extending UI without re-implementing existing controls

[dawolff-ms]

Changelog Entry

• Added ability for developers to customize Web Chat by extending the default UI without having to re-implement existing components. @dawolff-ms in PR #4539

Description

Adding the ability to extend the default UI for Web Chat without having to reimplement already existing controls. For example, our team wishes to add our own custom component betweeen the connectivity status and the send box. Currently, the only way to do this is to recompose the UI with a custom send box, which doesn't make much sense since we don't have a desire to change the send box functionality.

Design

The design is rather simple: export the AccessKeySinkSurface, BasicToaster, BasicTranscript, BasicConnectivityStatus, BasicSendBox. This allows developers to use them to build their own custom BasicWebChat-like component.

Specific Changes

1. I’ve exported all of the components the BasicWebChat control uses: AccessKeySinkSurface, BasicToaster, BasicTranscript, BasicConnectivityStatus, BasicSendBox. This allows us to use them in our own customizations.
2. I added a single test case that covers these changes.
3. I added a sample under "recomposing-ui" named "extending-ui" that outlines how to use this feature/change.
4. I ran tests locally and no (relevant) tests are failing with these changes.

PR Checklist

• I have added tests and executed them locally
• I have updated CHANGELOG.md
• I have updated documentation

Review Checklist

This section is for contributors to review your work.

• Accessibility reviewed (tab order, content readability, alt text, color contrast)
• Browser and platform compatibilities reviewed
• CSS styles reviewed (minimal rules, no z-index)
• Documents reviewed (docs, samples, live demo)
• Internationalization reviewed (strings, unit formatting)
• package.json and package-lock.json reviewed
• Security reviewed (no data URIs, check for nonce leak)
• Tests reviewed (coverage, legitimacy)

[cwhitten]

@dawolff-ms this is really well done! let's connect next week on a thorough review and ensure there are no regressions

[compulim]

@dawolff-ms this is excellent. I guess you have spent quite some time with Web Chat to gain certain knowledge to create this PR.

I am still reading this and digesting the information, here is my reading notes so far:

• You are spot on, the styles inside <BasicXXX> are not in their best places
  • You are doing a great job in this PR
  • Let me give you a future direction from things in my head:
    • Today, those styles are "hardcoded" today, because if web devs modify/override the style, it will break the UI very badly (say, forget to add display: flex)
    • We designed Web Chat in a way that web devs can throw away all CSS and the UI still "kind of work"
    • The separation between class name generated by hardcoded and style options are a bit confusing and a bit chores
    • Maybe we don't need to worry much about web devs and just put everything in the style set
• Related to <AccessKeySurfaceSink>
  • <Composer> is a context provider, which most devs expect them to be a headless component (no DOM)
  • The <AccessKeySurfaceSink> need to have a DOM element to capture keys
  • Moving <AccessKeySurfaceSink> into <Composer> means we turn the context provider into a headed component
  • I think we should not use DOM element in most context providers, unless they are optional and can be taken out
    • A new DOM element may not be desirable. Say, using <Composer> in React Native or other non-DOM environment

I will add more thoughts here after I fully read the PR.

Thanks for such a high quality PR.

[dawolff-ms]

@dawolff-ms this is excellent. I guess you have spent quite some time with Web Chat to gain certain knowledge to create this PR.

You are doing a great job in this PR

Thanks for such a high quality PR.

Thanks! Kudos to you and your team for maintaining such a clean and organized repo - working in it was super straight forward that putting together a PR like this was super easy!

• Today, those styles are "hardcoded" today, because if web devs modify/override the style, it will break the UI very badly (say, forget to add display: flex)
• We designed Web Chat in a way that web devs can throw away all CSS and the UI still "kind of work"
• The separation between class name generated by hardcoded and style options are a bit confusing and a bit chores
• Maybe we don't need to worry much about web devs and just put everything in the style set

I couldn't agree more, but this seems slightly out of scope for the changes I want to introduce. My goal was to change as little as possible to achieve this feature. If you'd like, I can remove the className props from the BasicTranscript and BasicSendBox if you're concerned about developers adding their own CSS.

• Related to <AccessKeySurfaceSink>

  • <Composer> is a context provider, which most devs expect them to be a headless component (no DOM)

  • The <AccessKeySurfaceSink> need to have a DOM element to capture keys

  • Moving <AccessKeySurfaceSink> into <Composer> means we turn the context provider into a headed component

  • I think we should not use DOM element in most context providers, unless they are optional and can be taken out

    • A new DOM element may not be desirable. Say, using <Composer> in React Native or other non-DOM environment

I wasn't aware of the headless aspect of the Composer - thanks for pointing this out. I actually already had my own concerns with modifying the Composer: any developers who are using it for customizations will automatically be forced to use the AccessKeySurfaceSink without any opt-in, surely causing trouble as it adds functionality that could clash with their customizations. Even if we were to introduce this, it shouldn't be introduced in a minor version.

Because of the above I'm ready to revert all changes I made to the Composer component, but that means we need to find another way to add AccessKeySurfaceSink back in. Here's the approach I'm thinking of: add AccessKeySurfaceSink back to the BasicWebChat component and export the AccessKeySurfaceSink from the package.

Pros:

• This allows us to include the AccessKeySurfaceSink manually when creating our own custom Web Chat component.
• This simplifies the changes needed for this PR.
• No backwards compatibility issues with existing customizations.

Cons:

• This starts to pollute your exports. Developers won't know what to do with a "AccessKeySurfaceSink" - perhaps we rename it or wrap it in something else (BasicWebChatContainer?).
• Not all developers may understand that they'll need to include this component in their customizations in order to achieve the same functionality as the BasicWebChat.

We can always lean on documentation to address the cons, I guess! Let me know what you think.

[dawolff-ms]

@compulim as per our discussion earlier, I've reverted all my changes and just have the exports for AccessKeySinkSurface, BasicToaster, BasicTranscript, BasicConnectivityStatus, BasicSendBox. I've updated the tests and sample to reflect the same.

Let me know if you see any issues.

[compulim]

I have added a screenshot.

Since our current CI pipeline is broken for forked branches, I am bypassing branch protection.

I have run all tests locally, added a new screenshot for the new test.