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

[P0] Send attachment on send #5123

Merged
merged 83 commits into from
Apr 8, 2024
Merged

Conversation

compulim
Copy link
Contributor

@compulim compulim commented Apr 6, 2024

Fixes #5083.

Changelog Entry

Breaking changes

  • useSendMessage hook is updated to support sending attachments with a message. To reduce complexity, the useSendFiles hook is being deprecated. The hook will be removed on or after 2026-04-03
  • styleOptions.uploadThumbnailHeight and styleOptions.uploadThumbnailWidth must be a number of pixels

Added

  • Resolves #5083. Added sendAttachmentOn style option to send attachments and text in a single activity, by @ms-jb and @compulim
    • useSendMessage hook is updated to support sending attachments with a message
    • useSendBoxAttachments hook is added to get/set attachments in the send box

Description

Added a new styleOption.sendAttachmentOn that enables attachments and message text to be sent in a single activity. If enabled, uploaded files are retained until the Send button is pressed to allow for message text to be optionally included. Standalone text works as before.

webchat-new-icons

Design

useSendFiles vs. useSendMessage

As useSendMessage hook now supports sending attachments in the message, the useSendFiles hook can be obsoleted.

We choose to add attachments to useSendMessage over useSendFiles because if we add text message to useSendFiles, it will become an uber hook and we would need to deprecate useSendMessage. This is undesirable.

sendBoxAttachments hook

New useSendBoxAttachments to get/set the attachments in the send box before it is being sent. This works nicely with speech recognition.

Simplifying attachments

Some philosophies we applied to simply the SDK design:

  • Userland attachment is { blob: Blob | File, thumbnailURL?: URL }, as minimal/intuitive as possible
    • Also extensible, say, { iconURL?: URL }
  • botframework-webchat-component will help making thumbnails when possible
    • useSendMessage will accept userland attachment. When the hook is called, thumbnails will be generated on-the-fly
    • useSendFiles will remain the same and accept Blob | File. When the hook is called, thumbnails will be generated on-the-fly
    • useSendBoxAttachments will accept userland attachment. However, it will not automatically generate thumbnails

Specific Changes

  • Hooks
    • Added useSendBoxAttachments hook to get/set attachments in the send box
    • Modified useSendMessage hook to add attachments
    • Deprecated useSendFiles hook
  • UI
    • Updated <UploadButton> to show checkmark when files are attached
    • Added styleOptions.sendAttachmentOn
      • "attach" will send the message with attachment when files are attached (legacy)
      • "send" will send the message with attachment when the send button is tapped (default)
  • 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)

@compulim compulim changed the title Feat pending upload 2 Send attachment on send Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p0 Must Fix. Release-blocker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability to combine attachments and message text in a single activity
3 participants