-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add "send failed retry" prompt to live region #4362
Merged
compulim
merged 52 commits into
microsoft:main
from
compulim:feat-a11y-send-failed-retry
Aug 3, 2022
Merged
Add "send failed retry" prompt to live region #4362
compulim
merged 52 commits into
microsoft:main
from
compulim:feat-a11y-send-failed-retry
Aug 3, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
compulim
requested review from
cwhitten,
srinaath,
tdurnford,
tonyanziano and
beyackle
as code owners
August 2, 2022 09:10
tdurnford
reviewed
Aug 2, 2022
tdurnford
reviewed
Aug 2, 2022
tdurnford
reviewed
Aug 2, 2022
tdurnford
reviewed
Aug 2, 2022
tdurnford
reviewed
Aug 2, 2022
tdurnford
reviewed
Aug 2, 2022
tdurnford
previously approved these changes
Aug 2, 2022
packages/api/src/providers/ActivitySendStatus/ActivitySendStatusComposer.tsx
Outdated
Show resolved
Hide resolved
packages/api/src/providers/ActivitySendStatus/ActivitySendStatusComposer.tsx
Outdated
Show resolved
Hide resolved
packages/component/src/Middleware/ActivityStatus/SendStatus/SendStatus.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: TJ Durnford <tjdford@gmail.com>
Co-authored-by: TJ Durnford <tjdford@gmail.com>
tdurnford
approved these changes
Aug 3, 2022
This was referenced Nov 11, 2022
Closed
11 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changelog Entry
Breaking changes
activity.channelData.state
property is being deprecated in favor of the updatedactivity.channelData['webchat:send-status']
property, main differences:'webchat:send-status'
property will become"send failed"
when the chat adapter failed to send the activity or after passing a hardcoded 5 minute timeoutstate
property would become"send failed"
when the chat adapter failed to send the activity or after passing a timeout as defined instyleOptions.sendTimeout
Changed
useSendStatusByActivityKey
hook to check the UI send status of an outgoing activity, by @compulim in PR #4362styleOptions.sendTimeout
prop, the send status returned by this hook may transition from"send failed"
to"sending"
, and vice versaactivity.channelData['webchat:send-status']
Fixed
Description
When an activity was failed to send (e.g. send timeout, network error, etc.), a "Send failed, retry" prompt will be shown on the chat history. However, this retry prompt is not narrated through live region.
This pull request is to add the retry prompt to the live region to make sure screen reader users are aware that the message they sent is failing and they can retry.
The "send failed" prompt will only read for failing message that are also non-presentational (e.g. not
event
ortyping
activities).While we are working on this PR, we discovered a few flaws in the current version of Web Chat. The flaws are documented in the design section of this PR description:
channelData.state === 'send failed'
was intentionally ignoredchannelData.state
set momentarilypostActivity()
call did not return yet, the activity-in-transit should still have send status ofsending
, instead of unsetcreateDirectLineWithTranscript
does not work properly for outgoing activities because thepostActivitySaga
is not triggeredchannelData.state
in thecreateDirectLineWithTranscript.activity$
to emulate the effect ofpostActivitySaga
, this behavior is not correctDesign
New
<ActivitySendStatus>
providerA new context provider to provide UI-oriented information on outgoing activity send status. Its corresponding hook
useSendStatusByActivityKey
will retrieve a map of send status by activity key. Its return value has type ofMap<string, 'sending' | 'send failed' | 'sent'>
.All existing code related to send status of outgoing activities are being refactored to use this new provider.
New code added to chat history live region will use this provider to make sure it has a consistent experience as the visible chat history.
This send status is different from the
channelData['webchat:send-status']
:channelData['webchat:send-status']
useSendStatusByActivityKey
"sending"
"sending"
if not timed out, otherwise,"send failed"
"send failed"
"send failed"
"sent"
"sent"
Notes: timeout is based on
styleOptions.sendTimeout
orstyleOptions.sendTimeoutForAttachments
.Web developers can change
styleOptions
to modify send timeout on-the-fly and theuseSendStatusByActivityKey
will reflect the change immediately.New
activity.channelData['webchat:send-status']
This is very similar to the existing
activity.channelData.state
with a few differences:webchat:
for better namespacesending
will becomesend failed
if and only if:styleOptions.sendTimeout
activity.channelData.state === 'send failed'
is not very helpful because:styleOptions.sendTimeout
is increased, it will NOT revive the status back tosending
because thepostActivitySaga
has already stoppedFor the reasons above, we ignored both
sending
andsend failed
state. We only considered thesent
and not-sent
statesent
state, we will mark the activity as "send failed"send failed
state. The UI will need to wait for 20s to reflect the fatal errorThe newer channel data field will speed up showing "Send failed" as soon as fatal error is detected.
This field is NOT recommended to be a factor of whether the "Send failed. Retry" prompt should be shown or not. Web developers should use the new
<ActivitySendStatus>
provider and itsuseSendStatusByActivityKey
hook. This is because this channel data field is not based onstyleOptions.sendTimeout
. Thus, it is not a direct representation of whether the "Send failed. Retry" prompt should be shown or not.Added test facility
createDirectLineEmulator
Previously, we use
createDirectLineWithTranscript
and inject static transcript withactivity.channelData.state
. However, this approach is not truly reflecting the reality:postActivitySaga.ts
channelData.state
Thus, we created a new
createDirectLineEmulator
:createDirectLineWithTranscript
, otherwise;createDirectLineEmulator
insteadFixing intermediate send status
Previously, when an outgoing activity is sent successfully, the
activities
reducer would receive the following actions:POST_ACTIVITY
, which triggers thepostActivitySaga
POST_ACTIVITY_PENDING
INCOMING_ACTIVITY
, when an echo back is received from the chat adapter/servicePOST_ACTIVITY_FULFILLED
When
INCOMING_ACTIVITY
is processed by theactivities
reducer, it upsert an activity into the state withchannelData.state
unset. This is because the incoming activity from the chat adapter/service should not contain this channel data field.However, after the
INCOMING_ACTIVITY
is handled by thetake
effect inpostActivitySaga
, thePOST_ACTIVITY_PENDING
is dispatched. ThePOST_ACTIVITY_PENDING
set thechannelData.state
to'sent'
for the activity in theactivities
state.Thus, the
channelData.state
transitioned fromsending
to unset tosent
in a very short period of time.The send status for outgoing activities should never be unset.
The following table explains differences between the current
channelData.state
and newerchannelData['webchat:send-status']
, in chronological order:state
after processwebchat:send-status
after processPOST_ACTIVITY
activities
state)activities
state)POST_ACTIVITY_PENDING
sending
sending
INCOMING_ACTIVITY
sending
POST_ACTIVITY_FULFILLED
sent
sent
For backward compatibility, the
channelData.state
field has not been updated. Instead, the newchannelData['webchat:send-status']
is updated so it will always have a value for all outgoing activities.Specific Changes
(Please see design section of this pull request)
CHANGELOG.md
Review Checklist
CSS styles reviewed (minimal rules, noz-index
)package.json
andpackage-lock.json
reviewedSecurity reviewed (no data URIs, check for nonce leak)