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

feat: New care group screen #76

Merged
merged 20 commits into from
Apr 19, 2024
Merged

feat: New care group screen #76

merged 20 commits into from
Apr 19, 2024

Conversation

haleymartin-6
Copy link
Contributor

Description

[Link to Ticket](insert the link to your ticket inside the parenthesis here)

Please include a summary of the changes and the related issue. Please also
include relevant motivation, context, and images!

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. If they are unit
tests, provide the file name the tests are in. If they are not unit tests,
describe how you tested the change.

Checklist

  • I have performed a self-review of my code
  • I have reached out to another developer to review my code
  • I have commented my code, particularly in hard-to-understand areas
  • New and existing unit tests pass locally with my changes
Screenshot 2024-04-08 at 7 27 53 PM

Copy link
Contributor

@MattCMcCoy MattCMcCoy left a comment

Choose a reason for hiding this comment

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

Amazing job Haley just some small things!

</View>
</View>

<View className="mx-2 mt-2 flex h-[180vh] w-[96vw] flex-col items-center rounded-xl">
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason for 180vh here? Scroll view should handle height on its own

Copy link
Contributor

Choose a reason for hiding this comment

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

removed

go.mod Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed.

Comment on lines 58 to 88
<ScrollView>
<View className="bg-carewallet-lightergray">
<View className="flex h-[10vh] items-center">
<View className="h-[10vh] scroll-pb-96">
<View className="flex w-full flex-row justify-end text-carewallet-black">
<Button
className="mx-auto mt-2 h-[50px] w-[96vw] items-start justify-center rounded-xl border-carewallet-lightgray bg-carewallet-white"
textColor="#000000"
mode="outlined"
icon={() => <Settings />} // Example using FontAwesome icon
>
<Text className="font-carewallet-manrope-semibold">
{' '}
Manage Caregiver Capabiities{' '}
</Text>
</Button>
</View>
</View>
</View>

<View className="mx-2 mt-2 flex h-[180vh] w-[96vw] flex-col items-center rounded-xl">
{users
.filter((user) => user.user_id !== activeUser)
.map((user) => (
<View
key={user.user_id} // <-- Add key prop here
onTouchEnd={() => {
navigation.navigate('Profile');
}}
>
<CareTaker
Copy link
Contributor

Choose a reason for hiding this comment

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

Noticed a scroll bug when I am scrolling and had pressed on a card it will visit the card after i lift up my finger. Look at other scroll views on how we handled this!

Copy link
Contributor

Choose a reason for hiding this comment

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

this part should be fixed, although it still will navigate to 'Profile' and not the profile of the person it touches. since the profile component doesn't take in a userId or anything not sure how to navigate to the profile of an actual user without refactoring the profile component. any ideas or is it okay as is? @MattCMcCoy

/>
</View>
))}
<View className="h-[70vh] pt-10"></View>
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
<View className="h-[70vh] pt-10"></View>

Copy link
Contributor

Choose a reason for hiding this comment

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

removed

Comment on lines 81 to 85
<CircleCard
Icon={<Settings />}
ButtonText="Settings"
onTouchEnd={() => navigation.navigate('CareGroup')}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

This button is nested in settings on main. So when fixing merge conflicts, let caplan or I know if you have any problems.

Copy link
Contributor

Choose a reason for hiding this comment

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

in my latest commit i will have allowed it to navigate from the settings page by pressing the 'manage caregiver capabilities' button

);
}
return (
<GestureHandlerRootView className="bg-carewallet-white pt-5">
Copy link
Contributor

Choose a reason for hiding this comment

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

What this for?

Copy link
Contributor

Choose a reason for hiding this comment

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

i have no idea but i removed it

Comment on lines 69 to 72
<Text className="font-carewallet-manrope-semibold">
{' '}
Manage Caregiver Capabiities{' '}
</Text>
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
<Text className="font-carewallet-manrope-semibold">
{' '}
Manage Caregiver Capabiities{' '}
</Text>
<Text className="font-carewallet-manrope-semibold">
Manage Caregiver Capabiities
</Text>


<View className="mx-2 mt-2 flex h-[180vh] w-[96vw] flex-col items-center rounded-xl">
{users
.filter((user) => user.user_id !== activeUser)
Copy link
Contributor

Choose a reason for hiding this comment

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

we should prob filter out patient as well

Copy link
Contributor

Choose a reason for hiding this comment

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

added

Comment on lines 84 to 86
onTouchEnd={() => {
navigation.navigate('Profile');
}}
Copy link
Contributor

Choose a reason for hiding this comment

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

is this able to set the active profile on profile page depending on who you click on?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see ignore my comment from before. I'm trying to do it like this but I'm assuming that the navigate function doesn't take into account the new state even if it was set differently again?:

onPress={() => { setActiveUser(user.user_id) navigation.navigate('Profile'); }}

Comment on lines +18 to +19
// const { user: signedInUser, group } = useCareWalletContext();
// const [activeUser, setActiveUser] = useState(signedInUser.userID);
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
// const { user: signedInUser, group } = useCareWalletContext();
// const [activeUser, setActiveUser] = useState(signedInUser.userID);

@MattCMcCoy
Copy link
Contributor

Once data branch is merged this is good to go 🚀 @narayansharma-21
#90

@MattCMcCoy MattCMcCoy merged commit 09525be into main Apr 19, 2024
3 checks passed
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.

3 participants