Skip to content

Conversation

@khushal87
Copy link
Member

@khushal87 khushal87 commented Oct 17, 2025

This pull request introduces significant improvements to message delivery status tracking and notification handling in the sample app and core package. The main focus is on refactoring how message delivery status is determined and displayed in channel previews, as well as improving notification handling for both foreground and background messages. The changes include a new hook for message delivery status, updates to type usage for better consistency, and improved notification display logic.

Message Delivery Status Refactor:

  • Introduced a new useMessageDeliveryStatus hook to determine and track the delivery status (SENT, DELIVERED, READ, or NOT_SENT_BY_CURRENT_USER) of the latest message in a channel, replacing the previous read status logic. This makes the status handling more granular and accurate.
  • Updated useLatestMessagePreview and related components to use the new MessageDeliveryStatus enum and hook, and refactored related types to use LocalMessage for better type safety and clarity.

Notification Handling Improvements:

  • Added a shared displayNotification utility to handle notification display logic in both foreground and background, reducing code duplication and improving maintainability.
  • Implemented a background message handler (bootstrapBackgroundMessageHandler.ts) that initializes the chat client, marks messages as delivered, and displays notifications when messages are received in the background.
  • Ensured the background message handler is imported and initialized at app startup.

General Code Quality and Consistency:

  • Improved type usage throughout the channel preview and message preview hooks, replacing ad-hoc types with LocalMessage for consistency and reliability.
  • Minor formatting and code style improvements for better readability and maintainability.

These changes collectively improve the reliability and user experience of message delivery indicators and push notifications in the application.

@Stream-SDK-Bot
Copy link
Contributor

Stream-SDK-Bot commented Oct 17, 2025

SDK Size

title develop branch diff status
js_bundle_size 291 KB 291 KB +423 B 🟡

import 'react-native-gesture-handler';
import { AppRegistry } from 'react-native';
import { enableScreens } from 'react-native-screens';
import './src/utils/bootstrapBackgroundMessageHandler';
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this before RNGH and all of the other imports please, it's important that the very first thing encountered whenever App.tsx is hit is the bootstrapping

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert these changes when you can, they do not seem correct

Copy link
Member Author

Choose a reason for hiding this comment

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

Well they are just the same with different indentation. This is a change that I open get on my system while opening Xcode and adding a setting. But, i will revert it back

const areEqual = (prevProps: MessagePropsWithContext, nextProps: MessagePropsWithContext) => {
const {
chatContext: { mutedUsers: prevMutedUsers },
deliveredBy: prevDeliveredBy,
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be deliveredTo or deliveredToCount or something similar ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I will change it to deliveredToCount


export const MessageStatus = (props: MessageStatusProps) => {
const { message, readBy, threadList } = useMessageContext();
const { channel } = useChannelContext();
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're only really using the channel to see if the members length is greater than 2, perhaps we could precalculate this elsewhere and just pass it to be used as an atomic value ? That way we won't have to worry about unstable references changing (nor probably need to run useChannelContext() like this downstream (which will trigger many changes). Having it here pre-memoization is definitely an improvement but we can still move it outside since we were anyway never passing it as a prop so it isn't breaking either.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well we do have channel as a direct prop to the MessageStatus component so I am not sure if we can do that.

Copy link
Contributor

@isekovanic isekovanic left a comment

Choose a reason for hiding this comment

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

Apart from the couple of comments I don't have any other concrete objections ! We have to refactor the ChannelPreview at some point, it feels like it's slowly getting out of hand and it could use some love.

Please profile the PR on both platforms to make sure we aren't degrading performance further (it doesn't look like it at first glance, but let's be sure of it)

@khushal87 khushal87 marked this pull request as ready for review October 24, 2025 06:20
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.

3 participants