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.

Sign up for GitHub

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

Jump to bottom

class based restapi migration for chat components #862

Closed
wants to merge 25 commits into from
Closed

class based restapi migration for chat components #862

wants to merge 25 commits into from

Conversation

KlausMikhaelson
Copy link
Contributor

No description provided.

pritipsingh
pritipsingh reviewed Nov 21, 2023
View reviewed changes
packages/uiweb/src/lib/components/chat/ChatViewBubble/ChatViewBubble.tsx Outdated
return (
<Section justifyContent="start" alignItems="start">
{chat.fromCAIP10.split(':')[1] !== account && (
{(chat.fromDID || chat.fromDID).split(':')[1] !== account && (
Copy link
Contributor

Choose a reason for hiding this comment

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

chat.fromDID || chat.fromDID aren't these 2 the same? what's the point of OR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, thanks for pointing out will remove one

@pritipsingh
Copy link
Contributor

Couple of isses:

  • If I change the chatid, despite the changes, the chats of the old chatid is sustained even though it's a chat id with different sets of people , prolly cause we're saving it in localstorage & not clearing it upon change of it. But could be something else

  • On loading, it starts on the top instead of scrolling down

  • If I am adding any wallet in the group, I am not receiving any request and I tried it doing from the Push daap and I received it.

  • this is difficult to reproduce but sometimes I get this error: Cannot read properties of undefined (reading 'content') at usePushChatSocket.ts:62:45

Screenshot 2023-11-21 at 5 36 23 PM

mishramonalisha76
mishramonalisha76 suggested changes Nov 21, 2023
View reviewed changes
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.

whereever you are checking
if(pushUser) use instead of pushUser use Object.keys(PushUser || {}).length
Because its an object.
Make a chat profile fetch hook

packages/uiweb/src/lib/components/chat/ChatViewBubble/ChatViewBubble.tsx Outdated
@@ -62,10 +63,10 @@ const SenderMessageProfilePicture = ({ chat }: { chat: IMessagePayload }) => {
};
useEffect(() => {
getUserPfp();
}, [account, chat.fromCAIP10]);
}, [account, chat.fromDID]);
Copy link
Contributor

Choose a reason for hiding this comment

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

lets revert this back to .fromCAIP10, in this file

packages/uiweb/src/lib/components/chat/ChatViewList/ChatViewList.tsx Outdated
});
if (result) {
newChatFeed = getDefaultFeedObject({ user: result });
console.log(signer)
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the console .log

packages/uiweb/src/lib/components/chat/ChatViewList/ChatViewList.tsx Outdated
}, [conversationHash, pgpPrivateKey, account, env,chatFeed]);
// if (conversationHash) {
(async function () {
console.log('chatHistory', pushUser)
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the console.log

packages/uiweb/src/lib/components/chat/ChatViewList/ChatViewList.tsx Outdated
threadHash,
});

if (pushUser && chatId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why did we change the condtion
Lets make it
if (
threadHash &&
((account && chatFeed && !chatFeed?.groupInformation) ||
(chatFeed && chatFeed?.groupInformation))
)
instead of
if (pushUser && chatId) {

packages/uiweb/src/lib/components/chat/ChatViewList/ChatViewList.tsx Outdated
return !!(
chatFeed &&
chatFeed?.groupInformation &&
!chatFeed?.groupInformation?.isPublic &&
((!isMember && pgpPrivateKey) || (!pgpPrivateKey))
(!isMember)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is for blurring out private chats, so if the user has not logged in yet we will show it as blurred to not show the decrypted chat. so the condition will be
(!isMember && pushUser)

packages/uiweb/src/lib/dataProviders/ChatDataProvider.tsx Outdated
Comment on lines 75 to 79
if (!pushUser && signer) {
console.log("userrr",user);
if (user) {
setPushUser(user);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

what does this condition mean

Copy link
Contributor

Choose a reason for hiding this comment

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

bff2e19#diff-09cd3b29295dd9aa7c4597a09653d6700b9bca9086110130257a85b6cf1e36a5

Follow this for chatDataProvider code

Copy link
Contributor

Choose a reason for hiding this comment

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

Current code is not right

packages/uiweb/src/lib/dataProviders/ChatDataProvider.tsx Outdated
Comment on lines 135 to 96
setConnectedProfile
setConnectedProfile,
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove connectedProfile

packages/uiweb/src/lib/helpers/chat/chat.ts Outdated
(walletToPCAIP10(account!).toLowerCase() ===
msg.fromCAIP10?.toLowerCase())))
msg.fromDID?.toLowerCase())))
Copy link
Contributor

Choose a reason for hiding this comment

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

replace fromDID to fromCAIP10

packages/uiweb/src/lib/hooks/chat/useChatProfile.ts
@@ -1,32 +1,41 @@
import * as PushAPI from '@pushprotocol/restapi';
Copy link
Contributor

Choose a reason for hiding this comment

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

if this hook is for initialisation, then it should be names something else like useInitializePushUser()

packages/uiweb/src/lib/hooks/chat/useChatProfile.ts Outdated
}

const useChatProfile = () => {
const { env } = useChatData();
const fetchUserChatProfile = useCallback(
const { signer, pushUser, setPushUser, setConnectedProfile, env } = useChatData();
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove setConnectedProfile

@github-actions GitHub Actions
Copy link

All looks good.

@mishramonalisha76 mishramonalisha76 linked an issue Jan 2, 2024 that may be closed by this pull request
😈 [Improvement Proposal] - Migration of chat components for ChatProfile based sdk #904
Closed
@mishramonalisha76 mishramonalisha76 changed the title added new restapi in uiweb chat components class based restapi migration for chat components Jan 3, 2024
@mishramonalisha76 mishramonalisha76 deleted the class-component-uiweb-refractored branch March 5, 2024 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pritipsingh pritipsingh pritipsingh left review comments

@mishramonalisha76 mishramonalisha76 mishramonalisha76 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

😈 [Improvement Proposal] - Migration of chat components for ChatProfile based sdk
3 participants
@KlausMikhaelson @pritipsingh @mishramonalisha76