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

Fixed Follow Button Of CurrentUser #531

Merged
merged 3 commits into from
Jan 28, 2025

Conversation

rahul31124
Copy link
Contributor

@rahul31124 rahul31124 commented Jan 24, 2025

This PR resolves two main issues in the Follow Card component:

Incorrect Text Display: The text displayed as "Followers" in both the "Followers" and "Following" sections, which caused confusion.

Follow Button Visibility Issue:
The Follow button appeared next to the user's own profile in the followers/followings list of another user
Changes Made:
Fixed Incorrect Text: The text now properly reflects whether the user is in the Followers or Following list.
The follow button will not appear next to the user's own profile in the followers/following list any other user.

Before
Follow_Issue_Before.webm
After
Follow_Issue_After.webm

07jasjeet

This comment was marked as outdated.

Copy link
Collaborator

@07jasjeet 07jasjeet left a comment

Choose a reason for hiding this comment

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

Hi @rahul31124, this task would require you to refactor every usage of follow button and hoist its state properly, please use view-models in high level components only. This for reasons already explained in review comment.

@@ -838,9 +841,11 @@ private fun FollowersCard(
}
}
Spacer(modifier = Modifier.height(10.dp))
val dashboardViewModel: DashBoardViewModel = hiltViewModel()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Injecting view-models into low level components hinders our ability to test and render previews because previews cannot instantiate view-models. You would probably need to get this view-model from a high level component (its usage site) and then hoist the isSelf state from there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@07jasjeet Hi Sir,I have implemented the State Hoist. Could you please check if everything looks good?

rahul31124 and others added 2 commits January 27, 2025 23:36
1. Remove view-model usage
2. Fix some UI issues
@07jasjeet
Copy link
Collaborator

Hi, I've pushed some changes to make things correct. Please go through the changes to derive knowledge of the codebase. Primarily, I want to focus change where I removed the view-model being used in a layer which is preview-able. This is a behaviour we want to achieve everytime and build towards it.

@07jasjeet 07jasjeet merged commit b02c7b0 into metabrainz:main Jan 28, 2025
1 check 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.

2 participants