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

Friends #48

Open
wants to merge 94 commits into
base: main
Choose a base branch
from
Open

Friends #48

wants to merge 94 commits into from

Conversation

sophiabrent
Copy link
Contributor

@sophiabrent sophiabrent commented Apr 19, 2023

Pulling friends logic from backend, along with updated UI.

Should:

  • Give user ability to see notification and decline/accept friend requests or group invites.
  • Show up to date list of friends
  • Allow users to search for, request, add, or un-add friends.

Still TODO:

  • Sorting notifications by time does not work currently
  • Refactoring friend search to use FlatList
  • Weird bug: cards are not showing correct status on search
  • Pages should re-render to show most up to date info on changes/navigation

src/App.tsx Outdated
@@ -89,6 +90,10 @@ const SocialScreenStack = () => {
name={SocialRoute.NOTIFICATIONS}
component={NotificationsScreen}
/>
<SocialStack.Screen
name={SocialRoute.FRIEND_PROFILE}
component={FriendProfileScreen}
Copy link
Contributor

Choose a reason for hiding this comment

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

Getting this TS warning here:
image

I think it has to do with the props on FriendProfileScreen:
image

Try using NativeStackScreenProps as type?

ios/Podfile.lock Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

errr is there a reason why Podfile.lock is changed? If there aren't any other dependencies being used, npm install again and then cd ios; pod install and hopefully all will be well :) also make sure your Cocoapods version is 1.12.0!

source={
imgPath
? { uri: `${imgPath}` }
: require('@nightlight/assets/images/anon.png')
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's start using initials instead of anon.png (see UserCircle.tsx)

src/components/social/NotificationCard.styles.ts Outdated Show resolved Hide resolved
src/components/social/NotificationCard.styles.ts Outdated Show resolved Hide resolved
src/screens/social/NotificationsScreen.tsx Outdated Show resolved Hide resolved
Comment on lines 25 to 36
let count = 0;
res.notifications.forEach(
(item: { data: { notificationType: string } }) => {
if (
item.data.notificationType === 'friendRequest' ||
item.data.notificationType === 'groupInvite'
) {
count++;
}
}
);
setCounter(count);
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be more efficient to use .filter() then .length :) You might also wanna consider defining an array of notification types to count [NotificationType.FriendRequest, NotificationType.GroupInvite], and simply check membership of each notification type using .includes().

Comment on lines 42 to 48
a.data.notificationType === 'friendRequest' ||
a.data.notificationType === 'groupInvite'
) {
return -1;
} else if (
b.data.notificationType === 'friendRequest' ||
b.data.notificationType === 'groupInvite'
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe consider using the membership check above?

src/screens/social/NotificationsScreen.tsx Outdated Show resolved Hide resolved
src/types.ts Outdated Show resolved Hide resolved
src/types.ts Outdated Show resolved Hide resolved
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.

4 participants