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

BA-1964: update unread badge and divider behavior #159

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
# @baseapp-frontend/components

## 0.0.39

### Patch Changes

- Improve unread Badge and unread divider behavior

## 0.0.38

### Patch Changes

- Add the ability to create new chat groups.
- Fix a few height bugs on Virtuoso lists.

- Updated dependencies
- @baseapp-frontend/design-system@0.0.26

Expand All @@ -32,7 +37,7 @@
### Patch Changes

- Updated dependencies
- @baseapp-frontend/graphql@1.1.14
- @baseapp-frontend/graphql@1.1.14

## 0.0.34

Expand Down
33 changes: 1 addition & 32 deletions packages/components/modules/messages/ChatRoom/index.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
'use client'

import { FC, Suspense, useEffect, useRef } from 'react'
import { FC, Suspense } from 'react'

import { useCurrentProfile } from '@baseapp-frontend/authentication'
import { LoadingState } from '@baseapp-frontend/design-system'

import { Box } from '@mui/material'
Expand All @@ -11,7 +10,6 @@ import { useLazyLoadQuery } from 'react-relay'
import { ChatRoomQuery as ChatRoomQueryType } from '../../../__generated__/ChatRoomQuery.graphql'
import DefaultMessagesList from '../MessagesList'
import DefaultSendMessage from '../SendMessage'
import { useReadMessageMutation } from '../graphql/mutations/ReadMessages'
import { ChatRoomQuery } from '../graphql/queries/ChatRoomQuery'
import ChatRoomHeader from './ChatRoomHeader'
import { ChatBodyContainer, ChatRoomContainer } from './styled'
Expand All @@ -24,7 +22,6 @@ const ChatRoom: FC<ChatRoomProps> = ({
SendMessage = DefaultSendMessage,
SendMessageProps = {},
}) => {
const hasRunRef = useRef(false)
// TODO: pre load this query and instead of lazyload
const { chatRoom } = useLazyLoadQuery<ChatRoomQueryType>(
ChatRoomQuery,
Expand All @@ -36,34 +33,6 @@ const ChatRoom: FC<ChatRoomProps> = ({
fetchKey: roomId,
},
)
const { currentProfile } = useCurrentProfile()
const [commitMutation] = useReadMessageMutation()

const prevProfileIdRef = useRef<string | undefined>(currentProfile?.id)
const prevRoomIdRef = useRef<string | undefined>(chatRoom?.id)

useEffect(() => {
if (hasRunRef.current)
return () => {
if (prevProfileIdRef.current && prevRoomIdRef.current) {
commitMutation({
variables: {
input: {
roomId: prevRoomIdRef.current,
profileId: prevProfileIdRef.current as string,
},
},
})
}
}
hasRunRef.current = true
return () => {}
}, [currentProfile?.id, chatRoom?.id])

useEffect(() => {
prevProfileIdRef.current = currentProfile?.id
prevRoomIdRef.current = chatRoom?.id
}, [currentProfile?.id, chatRoom?.id])

// TODO: handle error for chatRoom
if (!chatRoom) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const MessagesGroup: FC<MessagesGroupProps> = ({
message,
messageIndex,
isGroup = false,
firstUnreadMessageId,
MessageItem = DefaultMessageItem,
MessageItemProps = {},
}) => {
Expand Down Expand Up @@ -51,15 +52,9 @@ const MessagesGroup: FC<MessagesGroupProps> = ({

const renderUnreadMessagesDivider = useCallback(
(index: number) => {
const previousMessage = allMessages?.[index + 1]
const currentMessage = allMessages?.[index]
const hasPreviousMessage = !!previousMessage
const isFirstUnreadMessage =
currentMessage?.profile?.id !== currentProfile?.id &&
!currentMessage?.isRead &&
(!hasPreviousMessage || previousMessage?.isRead)

if (!!currentMessage && isFirstUnreadMessage) {
if (currentMessage?.id === firstUnreadMessageId) {
return (
<Divider
variant="fullWidth"
Expand All @@ -78,7 +73,7 @@ const MessagesGroup: FC<MessagesGroupProps> = ({

return null
},
[allMessages, currentProfile, theme.palette.error.light],
[allMessages, firstUnreadMessageId, theme.palette.error.light],
)

const renderLastMessageTime = useCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export interface MessagesGroupProps {
messageIndex: number
isGroup?: boolean
allMessagesLastIndex: number
firstUnreadMessageId?: string | null
hasNext: boolean
MessageItem?: FC<MessageItemProps>
MessageItemProps?: Partial<MessageItemProps>
Expand Down
40 changes: 39 additions & 1 deletion packages/components/modules/messages/MessagesList/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { FC, useCallback, useMemo, useRef } from 'react'
import { FC, useCallback, useEffect, useMemo, useRef } from 'react'

import { useCurrentProfile } from '@baseapp-frontend/authentication'
import { LoadingState } from '@baseapp-frontend/design-system'

import { Box } from '@mui/material'
Expand All @@ -8,6 +9,8 @@ import { Virtuoso, VirtuosoHandle } from 'react-virtuoso'

import { ChatRoomMessagesListPaginationQuery } from '../../../__generated__/ChatRoomMessagesListPaginationQuery.graphql'
import { MessagesListFragment$key } from '../../../__generated__/MessagesListFragment.graphql'
import { useChatRoom } from '../context'
import { useReadMessageMutation } from '../graphql/mutations/ReadMessages'
import { MessagesListFragment } from '../graphql/queries/MessagesList'
import useMessagesListSubscription from '../graphql/subscriptions/useMessagesListSubscription'
import DefaultMessagesGroup from './MessagesGroup'
Expand Down Expand Up @@ -40,6 +43,38 @@ const MessagesList: FC<MessagesListProps> = ({
const allMessagesLastIndex = Number(allMessages?.length) - 1
const firstItemIndex = Math.max(totalNumberOfMessages - allMessages.length, 0)

const { currentProfile } = useCurrentProfile()
const { id: selectedRoom } = useChatRoom()
const [commitMutation] = useReadMessageMutation()

const getFirstUnreadMessageId = () => {
if (room?.unreadMessages?.count === 0) return null
return allMessages.find((message, index) => {
const previousMessage = allMessages?.[index + 1]
const hasPreviousMessage = !!previousMessage
return (
message?.profile?.id !== currentProfile?.id &&
!message?.isRead &&
(!hasPreviousMessage || previousMessage?.isRead)
)
})?.id
}

const firstUnreadMessageId = useMemo(getFirstUnreadMessageId, [selectedRoom, currentProfile])

Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Dependencies for firstUnreadMessageId memoization need to be updated

The getFirstUnreadMessageId function uses room, allMessages, and currentProfile internally, but the current memoization only includes selectedRoom and currentProfile. Based on the code analysis:

  • allMessages is derived from room?.allMessages?.edges
  • The function checks room?.unreadMessagesCount
  • It uses message?.profile?.id and message?.isRead from allMessages

The memoization should include:

  • room?.unreadMessagesCount
  • allMessages
🔗 Analysis chain

Verify memoization dependencies for firstUnreadMessageId.

The current dependencies might not capture all relevant state changes.

Also applies to: 121-121


Let me gather more information about the dependencies used in the memoization.


Let me try a different approach to find the dependencies.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for state variables used in getFirstUnreadMessageId
rg -A 2 "const.*State|useState" packages/components/modules/messages/MessagesList/
# Search for variables used in the function
ast-grep --pattern 'const getFirstUnreadMessageId = () => { $$$  }'

Length of output: 1616


Script:

#!/bin/bash
# Search for allMessages and room variables
rg -B 5 "const \{.*allMessages.*\}" packages/components/modules/messages/MessagesList/
rg -B 5 "const \{.*room.*\}" packages/components/modules/messages/MessagesList/

# Search for any updates to these variables
ast-grep --pattern 'setAllMessages($$$)'
ast-grep --pattern 'setRoom($$$)'

Length of output: 249


Script:

#!/bin/bash
# Search for variable declarations and props destructuring
rg -B 2 "const.*=.*props" packages/components/modules/messages/MessagesList/
rg -B 2 "const \{.*\}.*=.*props" packages/components/modules/messages/MessagesList/

# Look for allMessages and room in the component
rg "allMessages|room" packages/components/modules/messages/MessagesList/index.tsx

# Check for any hooks that might update these values
rg "use\w+" packages/components/modules/messages/MessagesList/index.tsx

Length of output: 2482

useEffect(() => {
if (room?.unreadMessages?.count !== 0) {
commitMutation({
variables: {
input: {
roomId: room?.id as string,
profileId: currentProfile?.id as string,
},
},
})
}
}, [room?.id, room?.unreadMessages?.count, currentProfile])

useMessagesListSubscription(room?.id)

const renderLoadingState = () => {
Expand Down Expand Up @@ -72,6 +107,7 @@ const MessagesList: FC<MessagesListProps> = ({
messageIndex={messageIndex}
isGroup={room?.isGroup}
allMessagesLastIndex={allMessagesLastIndex}
firstUnreadMessageId={firstUnreadMessageId}
hasNext={hasNext}
{...MessagesGroupProps}
/>
Expand All @@ -83,6 +119,8 @@ const MessagesList: FC<MessagesListProps> = ({
firstItemIndex,
hasNext,
room?.isGroup,
firstUnreadMessageId,
room?.participants?.totalCount,
MessagesGroup,
MessagesGroupProps,
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const MessageCountUpdateSubscription = graphql`
chatRoomOnMessagesCountUpdate(profileId: $profileId) {
profile {
id
unreadMessagesCount
chatRooms {
totalCount
edges {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const newMessageSubscription = graphql`
`

const useMessagesListSubscription = (roomId: string) => {
// TODO: use current profile hook when it is available
const config = useMemo(() => {
const connectionID = ConnectionHandler.getConnectionID(roomId, 'chatRoom_allMessages')
return {
Expand Down
6 changes: 3 additions & 3 deletions packages/components/package.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
{
"name": "@baseapp-frontend/components",
"description": "BaseApp components modules such as comments, notifications, messages, and more.",
"version": "0.0.38",
"version": "0.0.39",
"main": "./index.ts",
"types": "dist/index.d.ts",
"sideEffects": false,
"scripts": {
"build": "rm -rf dist && tsc --build tsconfig.build.json",
"dev": "rm -rf dist && tsc --watch",
"build": "rm -rf dist && pnpm relay && tsc --build tsconfig.build.json",
"dev": "rm -rf dist && pnpm relay && tsc --watch",
"relay": "relay-compiler",
"relay-download-schema": "dotenv -- bash -c 'get-graphql-schema \"$NEXT_PUBLIC_RELAY_ENDPOINT\" > schema.graphql'",
"relay-update-schema": "pnpm relay-download-schema && pnpm relay",
Expand Down
Loading