Skip to content

Show count in bubble tag whenever a new notification is received#2015

Merged
rohitmalhotra1420 merged 3 commits intomainfrom
show-new-notif
Feb 12, 2025
Merged

Show count in bubble tag whenever a new notification is received#2015
rohitmalhotra1420 merged 3 commits intomainfrom
show-new-notif

Conversation

@riyanshu-patro
Copy link
Collaborator

@riyanshu-patro riyanshu-patro commented Feb 12, 2025

Pull Request Template

#1940

Description

Show a bubble in inbox sidebar menu, when a new notification is received and remove it once the user clicks on the inbox menu.

  • Problem/Feature:

Type of Change

  • Bug fix
  • New feature
  • Code refactor
  • Documentation update
  • Other (please describe):

Checklist

  • Quick PR: Is this a quick PR? Can be approved before finishing a coffee.
    • Quick PR label added
  • Not Merge Ready: Is this PR dependent on some other PR/tasks and not ready to be merged right now.
    • DO NOT Merge PR label added

Frontend Guidelines

Build & Testing

  • No errors in the build terminal
  • Engineer has tested the changes on their local environment
  • Engineer has tested the changes on deploy preview

Screenshots/Video with Explanation

  • Before: Explain the previous behavior

  • After: What's changed now

Additional Context

Review & Approvals

  • Self-review completed
  • Code review by at least one other engineer
  • Documentation updates if applicable

Notes

@github-actions
Copy link

In the file src/common/hooks/useInAppNotifications.tsx:

  1. Line 38: Missing closing curly brace after 'src::common::hooks::useStream::attachListeners::DISCONNECT::'.
  2. Line 40: Need to close the brace after setting isStreamConnected to false.
  3. Line 43: Missing closing parenthesis after userPushSDKInstance.
  4. Line 72: Missing closing curly brace after updatedMessages[data.chatId].slice(-5).
  5. Line 78: Need to close the brace after pushing data to updatedMessages[data.chatId].
  6. Line 106: Need to close the brace after setting updatedMessages.
  7. Line 133: Missing closing parenthesis after resetChatMessages.

In the file src/components/MobileNavButton.jsx:

  1. Line 83: Attributes are not correctly aligned in the <SelectedIcon> component.
  2. Line 86: Incorrectly placed attribute src={activeIcon} without proper structure.
  3. Line 131: Missing closing curly brace } after LeftBarSecondarySectionIcon.
  4. Line 133: Need to close the brace after LeftBarSecondaryItemIcon.
  5. Line 167: Missing closing parenthesis after height: 25px;.

In the file src/components/NavigationButton.jsx:

  1. Line 46: Missing closing parenthesis after {showMetamaskPushSnap}.
  2. Line 52: Incorrect structure inside the <Span> component, the closing parenthesis should be placed after the closing tag </Span>.
  3. Line 58: Missing closing curly brace } after NewTagNew.
  4. Line 60: Need to close the brace after hasMenuLogic: true.
  5. Line 85: Missing closing parenthesis after {showMetamaskPushSnap}.
  6. Line 91: Need to close the brace after setting font-weight.
  7. Lines 127-132: Commented out CSS styles are not properly commented.

In the file src/config/NavigationList.js:

  1. Line 27: Missing closing curly brace } at the end of secondary object.
  2. Line 45: Missing closing curly brace } at the end of headerTag object.
  3. Line 53: Incorrect structure for the channels object, need to close the object properly.
  4. Line 58: Incorrect structure for the Messaging object, need to close the object properly.
  5. Line 80: Need to close the brace after setting loading: true.

All looks good.

@github-actions
Copy link

github-actions bot commented Feb 12, 2025

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2025-02-12 10:34 UTC

@rohitmalhotra1420
Copy link
Contributor

Screenshot 2025-02-12 at 3 25 35 PM @riyanshu-patro please fix this alignment.

@github-actions
Copy link

In the code provided for File: src/common/hooks/useInAppNotifications.tsx

  1. There are several typos:
    • In line 16, 'useDeviceWidthCheck' should be 'useDeviceSizeCheck'.
    • In line 29, 'InAppChannelNotifications' should be imported from the correct path, and same for 'InAppChatNotifications' on line 30.
    • In line 36, there is a closing } missing after 'DISCONNECT' listener logic.
    • In line 39, there is a missing console statement after 'DISCONNECT' event.
    • In line 44, there is a missing closing } for the condition when UPDATED_MESSAGES[data.chatId].length > 5 is true.
    • In line 64, the 'onAutoClose' should be 'autoClose' for the notification component.
  2. The logic for slicing the chat messages to a maximum of 5 messages is not working as intended due to missing braces in the condition for 'data.event === 'chat.request'. This should be updated to: { if (updatedMessages[data.chatId].length && data.event === 'chat.request') {`.

In the code provided for File: src/components/MobileNavButton.jsx

  1. There is a typo in line 73, 'flex-direction' should be 'display'.
  2. There is a missing closing brace at the end of the 'NewTag' styled component.
  3. There is a missing closing brace at the end of the 'NewTag2' styled component.

In the code provided for File: src/components/NavigationButton.jsx

  1. There is a typo in line 80, 'LOADER_TYPE' should be imported from a specific location.
  2. There are several typos and logic issues:
    • In line 98, there is a missing break; after assigning values to 'SelectedIcon' and 'definedMargin' for 'GLOBALS.CONSTANTS.NAVBAR_SECTIONS.MOBILE' case.
    • In line 100, there is a missing break; after assigning values to 'SelectedIcon' and 'definedMargin' for 'GLOBALS.CONSTANTS.NAVBAR_SECTIONS.NOTIFICATION' case.
    • In line 136, onClick={data?.hasOnClickFunction && showMetamaskPushSnap} should be inside a conditional rendering based on 'active' state.
  3. There is a missing closing tag '' at line 140.
  4. There is a typo in line 175, 'BorderRadius' should be 'borderRadius' in 'LeftBarPrimaryItemIcon' styled component.
  5. There are missing closing braces for components styled components 'NewTag', 'NewTag2', and 'ProtectedRoute'.

That's all from my code review.

);
if (data.source != 'PUSH_CHAT')
if (data.source != 'PUSH_CHAT') {
setNewNotifsCount((prev: any) => prev + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
setNewNotifsCount((prev: any) => prev + 1);
setNewNotifsCount((prev: number) => prev + 1);

updatedMessages[data.chatId].push(data);
}
setNewMessages(updatedMessages);
setNewChatsCount((prev: any) => prev + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
setNewChatsCount((prev: any) => prev + 1);
setNewChatsCount((prev: number) => prev + 1);

Comment on lines +648 to +651
...prev,
notificationList: prev.notificationList.map((item: any) =>
item.id === '2_inbox' ? { ...item, data: { ...item.data, count: newNotifsCount } } : item
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not create a common function for this as its being used in 2 files.

useEffect(() => {
if (navigationSetup) {
setNavigationSetup((prev: any) => ({
...prev,
Copy link
Contributor

Choose a reason for hiding this comment

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

common function

@github-actions
Copy link

In the file src/common/Common.utils.tsx:

  1. In the function convertTimeStamp, there is a missing closing brace '}' after the condition if (diffInSeconds < 60). It should be added before the next 'else if' statement.
  2. In the function convertTimeStamp, there is a missing closing brace '}' after the condition if (updatedMessages[data.chatId].length > 5). It should be added before the next 'if' statement.
  3. There is a missing return statement in the function convertTimeStamp after the condition else if (diffInMinutes < 60).
  4. In the function getNotificationCount, the closing brace '}' is missing at the end of the function.
  5. In the function isValidURL, there is a missing closing brace '}' at the end of the function.
  6. In the function getCurrentEnv, there is a missing closing brace '}' at the end of the function.

In the file src/common/hooks/useInAppNotifications.tsx:

  1. Inside the attachListeners function, the first event handler for CONSTANTS.STREAM.DISCONNECT is missing console statement after the comment. It should be corrected.
  2. In the attachListeners function, the closing brace '}' is missing at the end of the function.
  3. In the resetChatMessages function, the closing brace '}' is missing before the last statement.
  4. In the streamAttach function, there is a missing closing brace '}' at the end of the function.
  5. In the streamCleanup function, there is a missing closing brace '}' at the end of the function.

In the file src/components/MobileNavButton.jsx:

  1. In the styled component NewTag, there is a missing closing brace '}' at the end of the component definition.
  2. In the styled component NewTag2, there is a missing closing brace '}' at the end of the component definition.

The rest of the files are not included for review.

Please review and make the necessary corrections. Let me know if you need any further assistance with this code.

@rohitmalhotra1420 rohitmalhotra1420 merged commit 4c34357 into main Feb 12, 2025
2 checks passed
@riyanshu-patro riyanshu-patro deleted the show-new-notif branch February 12, 2025 10:34
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.

Bubble tag in sidebar inbox menu, whenever a new notification is received

2 participants