Skip to content

Commit 3671c3d

Browse files
authored
fix: suppress upload cancelled notification for async uploads (#3693)
## 🎯 Goal This PR fixes a small UI glitch which would trigger an upload failure snackbar notification if we eagerly remove an attachment from the composer while it hasn't finished uploading (basically the entire usecase of `allowSendBeforeAttachmentsUpload`). We simply suppress these notifications as they make no sense and look weird on the UI. We however still trigger actual upload failures even if it's in the premature phase (composer) as they'll update the attachment regardless so that the user can retry. ## 🛠 Implementation details <!-- Provide a description of the implementation --> ## 🎨 UI Changes <!-- Add relevant screenshots --> <details> <summary>iOS</summary> <table> <thead> <tr> <td>Before</td> <td>After</td> </tr> </thead> <tbody> <tr> <td> <!--<img src="" /> --> </td> <td> <!--<img src="" /> --> </td> </tr> </tbody> </table> </details> <details> <summary>Android</summary> <table> <thead> <tr> <td>Before</td> <td>After</td> </tr> </thead> <tbody> <tr> <td> <!--<img src="" /> --> </td> <td> <!--<img src="" /> --> </td> </tr> </tbody> </table> </details> ## 🧪 Testing <!-- Explain how this change can be tested (or why it can't be tested) --> ## ☑️ Checklist - [ ] I have signed the [Stream CLA](https://docs.google.com/forms/d/e/1FAIpQLScFKsKkAJI7mhCr7K9rEIOpqIDThrWxuvxnwUq2XkHyG154vQ/viewform) (required) - [ ] PR targets the `develop` branch - [ ] Documentation is updated - [ ] New code is tested in main example apps, including all possible scenarios - [ ] SampleApp iOS and Android - [ ] Expo iOS and Android
1 parent 6e234bf commit 3671c3d

5 files changed

Lines changed: 103 additions & 6 deletions

File tree

package/src/components/MessageList/MessageFlashList.tsx

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ import { MessageInputHeightState } from '../../state-store/message-input-height-
6060
import { primitives } from '../../theme';
6161
import { transitions } from '../../utils/animations/transitions';
6262
import { MessageWrapper } from '../Message/MessageItemView/MessageWrapper';
63+
import { excludeCanceledUploadNotifications } from '../Notifications/notificationFilters';
6364
import { PortalWhileClosingView } from '../UIComponents/PortalWhileClosingView';
6465

6566
type FlashListContextApi = { getRef?: () => FlashListRef<LocalMessage> | null } | undefined;
@@ -139,7 +140,10 @@ type MessageFlashListPropsWithContext = Pick<
139140
| 'maximumMessageLimit'
140141
> &
141142
Pick<ChatContextValue, 'client'> &
142-
Pick<MessageInputContextValue, 'messageInputFloating' | 'messageInputHeightStore'> &
143+
Pick<
144+
MessageInputContextValue,
145+
'allowSendBeforeAttachmentsUpload' | 'messageInputFloating' | 'messageInputHeightStore'
146+
> &
143147
Pick<PaginatedMessageListContextValue, 'loadMore' | 'loadMoreRecent'> &
144148
Pick<
145149
MessagesContextValue,
@@ -259,6 +263,7 @@ const MessageFlashListWithContext = (props: MessageFlashListPropsWithContext) =>
259263
? InlineLoadingMoreRecentThreadIndicator
260264
: InlineLoadingMoreRecentIndicator;
261265
const {
266+
allowSendBeforeAttachmentsUpload,
262267
attachmentPickerStore,
263268
additionalFlashListProps,
264269
channel,
@@ -1153,7 +1158,10 @@ const MessageFlashListWithContext = (props: MessageFlashListPropsWithContext) =>
11531158
<AutoCompleteSuggestionList />
11541159
</PortalWhileClosingView>
11551160
</Animated.View>
1156-
<NotificationList bottomOffset={messageInputFloating ? messageInputHeight + 16 : undefined} />
1161+
<NotificationList
1162+
bottomOffset={messageInputFloating ? messageInputHeight + 16 : undefined}
1163+
filter={allowSendBeforeAttachmentsUpload ? excludeCanceledUploadNotifications : undefined}
1164+
/>
11571165
</View>
11581166
);
11591167
};
@@ -1238,11 +1246,13 @@ export const MessageFlashList = (props: MessageFlashListProps) => {
12381246
const { loadMore, loadMoreRecent } = usePaginatedMessageListContext();
12391247
const { loadMoreRecentThread, loadMoreThread, thread, threadInstance } = useThreadContext();
12401248
const { readEvents } = useOwnCapabilitiesContext();
1241-
const { messageInputFloating, messageInputHeightStore } = useMessageInputContext();
1249+
const { allowSendBeforeAttachmentsUpload, messageInputFloating, messageInputHeightStore } =
1250+
useMessageInputContext();
12421251

12431252
return (
12441253
<MessageFlashListWithContext
12451254
{...{
1255+
allowSendBeforeAttachmentsUpload,
12461256
attachmentPickerStore,
12471257
channel,
12481258
channelUnreadStateStore,

package/src/components/MessageList/MessageList.tsx

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ import { primitives } from '../../theme';
7474
import { transitions } from '../../utils/animations/transitions';
7575
import { useIncomingMessageAnnouncements } from '../Accessibility/hooks/useIncomingMessageAnnouncements';
7676
import { MessageWrapper } from '../Message/MessageItemView/MessageWrapper';
77+
import { excludeCanceledUploadNotifications } from '../Notifications/notificationFilters';
7778
import { PortalWhileClosingView } from '../UIComponents';
7879

7980
// This is just to make sure that the scrolling happens in a different task queue.
@@ -229,7 +230,10 @@ type MessageListPropsWithContext = Pick<
229230
MessagesContextValue,
230231
'disableTypingIndicator' | 'FlatList' | 'myMessageTheme' | 'shouldShowUnreadUnderlay'
231232
> &
232-
Pick<MessageInputContextValue, 'messageInputFloating' | 'messageInputHeightStore'> &
233+
Pick<
234+
MessageInputContextValue,
235+
'allowSendBeforeAttachmentsUpload' | 'messageInputFloating' | 'messageInputHeightStore'
236+
> &
233237
Pick<
234238
ThreadContextValue,
235239
'loadMoreRecentThread' | 'loadMoreThread' | 'threadHasMore' | 'thread' | 'threadInstance'
@@ -320,6 +324,7 @@ const MessageListWithContext = (props: MessageListPropsWithContext) => {
320324
? InlineLoadingMoreRecentThreadIndicator
321325
: InlineLoadingMoreRecentIndicator;
322326
const {
327+
allowSendBeforeAttachmentsUpload,
323328
animateLayout = true,
324329
attachmentPickerStore,
325330
additionalFlatListProps,
@@ -1391,7 +1396,10 @@ const MessageListWithContext = (props: MessageListPropsWithContext) => {
13911396
<AutoCompleteSuggestionList />
13921397
</PortalWhileClosingView>
13931398
</Animated.View>
1394-
<NotificationList bottomOffset={messageInputFloating ? messageInputHeight + 16 : undefined} />
1399+
<NotificationList
1400+
bottomOffset={messageInputFloating ? messageInputHeight + 16 : undefined}
1401+
filter={allowSendBeforeAttachmentsUpload ? excludeCanceledUploadNotifications : undefined}
1402+
/>
13951403
</View>
13961404
);
13971405
};
@@ -1423,14 +1431,16 @@ export const MessageList = (props: MessageListProps) => {
14231431
const { readEvents } = useOwnCapabilitiesContext();
14241432
const { disableTypingIndicator, FlatList, myMessageTheme, shouldShowUnreadUnderlay } =
14251433
useMessagesContext();
1426-
const { messageInputFloating, messageInputHeightStore } = useMessageInputContext();
1434+
const { allowSendBeforeAttachmentsUpload, messageInputFloating, messageInputHeightStore } =
1435+
useMessageInputContext();
14271436
const { loadMore, loadMoreRecent, hasMore } = usePaginatedMessageListContext();
14281437
const { loadMoreRecentThread, loadMoreThread, threadHasMore, thread, threadInstance } =
14291438
useThreadContext();
14301439

14311440
return (
14321441
<MessageListWithContext
14331442
{...{
1443+
allowSendBeforeAttachmentsUpload,
14341444
attachmentPickerStore,
14351445
channel,
14361446
channelUnreadStateStore,
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import type { Notification } from 'stream-chat';
2+
3+
import { excludeCanceledUploadNotifications } from '../notificationFilters';
4+
5+
const buildNotification = (overrides: Partial<Notification> = {}): Notification =>
6+
({
7+
createdAt: 0,
8+
id: 'n1',
9+
message: 'Error uploading attachment',
10+
origin: { emitter: 'AttachmentManager' },
11+
type: 'api:attachment:upload:failed',
12+
...overrides,
13+
}) as Notification;
14+
15+
const canceledError = (name: 'AbortError' | 'CanceledError') => {
16+
const error = new Error('canceled');
17+
error.name = name;
18+
return error;
19+
};
20+
21+
// A filter returns `true` to KEEP a notification, `false` to hide it.
22+
describe('excludeCanceledUploadNotifications', () => {
23+
it.each(['AbortError', 'CanceledError'] as const)(
24+
'hides an upload-failed notification aborted with %s',
25+
(name) => {
26+
const notification = buildNotification({ originalError: canceledError(name) });
27+
28+
expect(excludeCanceledUploadNotifications(notification)).toBe(false);
29+
},
30+
);
31+
32+
it('keeps a genuine upload failure', () => {
33+
const notification = buildNotification({ originalError: new Error('Network Error') });
34+
35+
expect(excludeCanceledUploadNotifications(notification)).toBe(true);
36+
});
37+
38+
it('keeps an upload-failed notification with no original error', () => {
39+
expect(excludeCanceledUploadNotifications(buildNotification())).toBe(true);
40+
});
41+
42+
it('keeps a cancellation error reported on a different notification type', () => {
43+
const notification = buildNotification({
44+
originalError: canceledError('CanceledError'),
45+
type: 'api:message:send:failed',
46+
});
47+
48+
expect(excludeCanceledUploadNotifications(notification)).toBe(true);
49+
});
50+
});

package/src/components/Notifications/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@ export * from './Notification';
22
export * from './NotificationList';
33
export * from './NotificationTargetContext';
44
export * from './hooks';
5+
export * from './notificationFilters';
56
export * from './notificationTarget';
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import type { Notification } from 'stream-chat';
2+
3+
import type { NotificationListFilter } from './NotificationList';
4+
5+
const ATTACHMENT_UPLOAD_FAILED_TYPE = 'api:attachment:upload:failed';
6+
7+
/**
8+
* A cancelled upload surfaces as an `api:attachment:upload:failed` notification
9+
* whose original error is an `AbortError` (fetch) or `CanceledError` (axios).
10+
* This is what happens when the user removes an attachment before its upload
11+
* has finished - the inflight request is aborted, not genuinely failed.
12+
*/
13+
const isCanceledAttachmentUpload = (notification: Notification) =>
14+
notification.type === ATTACHMENT_UPLOAD_FAILED_TYPE &&
15+
(notification.originalError?.name === 'CanceledError' ||
16+
notification.originalError?.name === 'AbortError');
17+
18+
/**
19+
* Notification list filter that keeps every notification except the spurious
20+
* "upload failed" one produced when a user removes an attachment mid-upload
21+
* (a filter returns `true` to keep a notification). Genuine upload failures
22+
* (network, server) are kept because their error is neither aborted nor
23+
* cancelled.
24+
*/
25+
export const excludeCanceledUploadNotifications: NotificationListFilter = (notification) =>
26+
!isCanceledAttachmentUpload(notification);

0 commit comments

Comments
 (0)