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

🐛 [BUG] - <Increase bg blur when group/chat is encrypted> #1312

Merged

Conversation

corlard3y
Copy link
Collaborator

@corlard3y corlard3y commented May 22, 2024

fix #1307
Increase bg blur when group/chat is encrypted

Fixes Issue

Changes proposed

Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • My change requires changes to the documentation.
  • I have updated the documentation accordingly.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

Screenshots

Note to reviewers

@corlard3y corlard3y linked an issue May 22, 2024 that may be closed by this pull request
Copy link

In the ChatViewList.tsx file:

  1. There is a missing closing parenthesis after the declaration of the &::-webkit-scrollbar CSS rule in the ChatViewListCard styled component.

  2. The margin prop in the Section component is being accessed with optional chaining (?.) operator, but it seems the position and theme.margin are assumed to be always present based on the code structure. It should be checked if these values are always defined before using the optional chaining.

  3. The ChatViewBubble component is being assigned a key prop with the same value as the index within a loop. This is not recommended as it may lead to unexpected behavior. It's better to use a unique key for each item in the list.

  4. In the ChatViewBubble component, the isGroup prop is being defined using the nullish coalescing operator (??). It might be more appropriate to verify if the value is explicitly null or undefined before using the operator.

  5. The ActionRequestBubble component is conditionally rendered based on initialized.chatInfo?.list === 'REQUESTS', ensure this logic aligns with the requirements.

Overall, the structure of the code seems fine, but it needs some corrections as mentioned above.

// Corrected Code snippet
const ChatViewListCard = styled(Section)<IThemeProps>`
  &::-webkit-scrollbar-thumb {
    background: ${(props) => props.theme.scrollbarColor};
    border-radius: 10px;
  }
  &::-webkit-scrollbar {
    width: 5px;
    overscroll-behavior: contain;
    scroll-behavior: smooth;
  }
`;

// Check initialization of position and theme.margin before using optional chaining
margins={position && theme.margin && position ? theme.margin.chatBubbleSenderMargin : theme.margin.chatBubbleReceiverMargin}

// Consider using a unique key instead of 'index' in the map function
{chatChunks.map((chat, index) => (
  <Section
    margin={position ? theme.margin.chatBubbleSenderMargin : theme.margin.chatBubbleReceiverMargin}
    key={index}
  >
    <ChatViewBubble
      decryptedMessagePayload={chat}
      key={index}
      isGroup={initialized.chatInfo?.meta?.group || false}
    />
  </Section>
))}

// Validate the condition for rendering ActionRequestBubble component
{initialized.chatInfo && initialized.chatInfo.list === 'REQUESTS' && (
  <ActionRequestBubble chatInfo={initialized.chatInfo} />
)}

Please review these corrections and make changes accordingly.

Copy link
Contributor

@mishramonalisha76 mishramonalisha76 left a comment

Choose a reason for hiding this comment

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

LGTM

@corlard3y corlard3y changed the base branch from main to alpha May 24, 2024 11:23
Copy link

  • In the useEffect hook with the dependency array [initialized.loading], there is a missing closing curly brace after the async function statement. It should be added before the }, [initialized.loading]);.

  • Inside the useEffect hook with the dependency array [chatAcceptStream, participantJoinStream], the comment mentioning "add comment" seems unnecessary and can be removed.

  • There is a missing closing curly brace before the if (Object.keys(participantRemoveStream || {}).length > 0 statement. It should be added after the }, [participantJoinStream]);.

  • Inside the transformSteamMessage function, there is a missing closing curly brace for the if (!user) { condition. It should be added before the subsequent if (initialized.chatInfo && (item?.chatId === initialized.chatInfo?.chatId || checkIfNewRequest(item, chatId))) {.

  • Additionally, there are indentation issues in the JSX rendering section within the ChatViewList component. Proper indentation should be maintained for readability and clarity.

After addressing the above issues, the code should be more structured and coherent.

@rohitmalhotra1420 rohitmalhotra1420 added Quick PR A PR that can be approved before finishing a coffee DO NOT MERGE If the PR/task is dependent on some other task add this label to the PR. labels May 29, 2024
@corlard3y corlard3y removed the DO NOT MERGE If the PR/task is dependent on some other task add this label to the PR. label May 29, 2024
@corlard3y corlard3y merged commit d8b026f into alpha May 29, 2024
1 check passed
@corlard3y corlard3y deleted the 1307-bug-increase-bg-blur-when-groupchat-is-encrypted branch May 29, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Quick PR A PR that can be approved before finishing a coffee
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 [BUG] - <Increase bg blur when group/chat is encrypted>
3 participants