-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
Updated FollowButton with "unfollow" design #22057
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
apps/admin-x-activitypub/src/api/activitypub.tsOops! Something went wrong! :( ESLint: 8.44.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "/apps/admin-x-activitypub".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react-hooks" was referenced from the config file in "apps/admin-x-activitypub/.eslintrc.cjs". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. apps/admin-x-activitypub/src/components/Profile.tsxOops! Something went wrong! :( ESLint: 8.44.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "/apps/admin-x-activitypub".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react-hooks" was referenced from the config file in "apps/admin-x-activitypub/.eslintrc.cjs". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. apps/admin-x-activitypub/src/components/Search.tsxOops! Something went wrong! :( ESLint: 8.44.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "/apps/admin-x-activitypub".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react-hooks" was referenced from the config file in "apps/admin-x-activitypub/.eslintrc.cjs". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
WalkthroughThis pull request introduces enhancements to the ActivityPub functionality in the admin-x package. The changes primarily focus on adding an unfollow feature to the ActivityPub API, updating the Changes
Sequence DiagramsequenceDiagram
participant User
participant FollowButton
participant ActivityPubAPI
participant QueryClient
User->>FollowButton: Click Unfollow
FollowButton->>ActivityPubAPI: unfollow(username)
ActivityPubAPI-->>FollowButton: Return Actor
FollowButton->>QueryClient: Invalidate following queries
QueryClient->>User: Update UI
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ref https://linear.app/ghost/issue/AP-311/ui-to-unfollow-an-actor - Added a confirmation modal on unfollow - Made the button in "Following" state more subtle - Made the label switch from "Following" to "Unfollow" on hover so it's clearer what it does
ref https://linear.app/ghost/issue/AP-311/ui-to-unfollow-an-actor - Refactored and simplified FollowButton so it only has 2 variants: primary (used on profiles, where it's the primary focus of the screen) and secondary (used in lists where there will probably be lots of FollowButtons next to each other) - Fixed import order in useActivityPubQueries.ts
ref https://linear.app/ghost/issue/AP-311 This is very similar to the /follow API so we've mostly copied the existing patterns there.
abc70fd
to
c58c150
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/admin-x-activitypub/src/components/global/FollowButton.tsx (1)
Line range hint
28-53
: Add confirmation modal for unfollow action.The PR objectives mention adding a confirmation modal for the unfollow action, but it's not implemented in the current code.
Consider adding a confirmation dialog before executing the unfollow mutation:
const handleClick = async () => { if (isFollowing) { + const confirmed = await NiceModal.show(ConfirmationModal, { + title: 'Unfollow Account', + prompt: `Are you sure you want to unfollow ${handle}?`, + okLabel: 'Unfollow', + cancelLabel: 'Cancel' + }); + if (!confirmed) { + return; + } setIsFollowing(false); onUnfollow(); unfollowMutation.mutate(handle); } else { setIsFollowing(true); onFollow(); followMutation.mutate(handle); } };
🧹 Nitpick comments (1)
apps/admin-x-activitypub/src/hooks/useActivityPubQueries.ts (1)
207-251
: Well-structured implementation of useUnfollow hook.The implementation:
- Follows React Query patterns
- Handles state updates comprehensively
- Includes proper error handling
- Maintains consistency with useFollow implementation
However, consider adding optimistic updates to improve perceived performance.
Here's how you could add optimistic updates:
export function useUnfollow(handle: string, onSuccess: () => void, onError: () => void) { const queryClient = useQueryClient(); return useMutation({ async mutationFn(username: string) { const siteUrl = await getSiteUrl(); const api = createActivityPubAPI(handle, siteUrl); return api.unfollow(username); }, + onMutate(fullHandle) { + // Cancel outgoing refetches + queryClient.cancelQueries([`profile:${fullHandle}`]); + queryClient.cancelQueries(['following:index']); + queryClient.cancelQueries(['follows:index:following']); + queryClient.cancelQueries(['followingCount:index']); + + // Snapshot previous values + const previousProfile = queryClient.getQueryData([`profile:${fullHandle}`]); + const previousFollowing = queryClient.getQueryData(['following:index']); + const previousFollows = queryClient.getQueryData(['follows:index:following']); + const previousCount = queryClient.getQueryData(['followingCount:index']); + + // Optimistically update + // ... (same updates as in onSuccess) + + return { previousProfile, previousFollowing, previousFollows, previousCount }; + }, onSuccess(unfollowedActor, fullHandle) { // ... existing code ... }, - onError + onError(_error, _fullHandle, context) { + // Revert optimistic updates on error + if (context) { + const { previousProfile, previousFollowing, previousFollows, previousCount } = context; + if (previousProfile) { + queryClient.setQueryData([`profile:${_fullHandle}`], previousProfile); + } + if (previousFollowing) { + queryClient.setQueryData(['following:index'], previousFollowing); + } + if (previousFollows) { + queryClient.setQueryData(['follows:index:following'], previousFollows); + } + if (previousCount) { + queryClient.setQueryData(['followingCount:index'], previousCount); + } + } + onError(); + } }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/admin-x-activitypub/package.json
(1 hunks)apps/admin-x-activitypub/src/api/activitypub.ts
(2 hunks)apps/admin-x-activitypub/src/components/Profile.tsx
(3 hunks)apps/admin-x-activitypub/src/components/Search.tsx
(2 hunks)apps/admin-x-activitypub/src/components/activities/ActivityItem.tsx
(1 hunks)apps/admin-x-activitypub/src/components/global/FollowButton.tsx
(3 hunks)apps/admin-x-activitypub/src/components/modals/ViewProfileModal.tsx
(3 hunks)apps/admin-x-activitypub/src/hooks/useActivityPubQueries.ts
(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/admin-x-activitypub/package.json
- apps/admin-x-activitypub/src/components/activities/ActivityItem.tsx
🔇 Additional comments (14)
apps/admin-x-activitypub/src/components/global/FollowButton.tsx (3)
1-12
: LGTM! Clean interface changes for the new design.The updated imports and interface changes effectively support the new hover state and styling requirements.
Line range hint
14-27
: LGTM! Well-structured state management.The isHovered state and secondary default type effectively implement the subtle design requirement.
60-81
: LGTM! Clean implementation of hover effects and styling.The dynamic label and styling changes effectively implement the hover state and subtle design requirements.
apps/admin-x-activitypub/src/components/Search.tsx (3)
60-63
: LGTM! Clean layout update.The flex column layout improves the visual hierarchy of account information.
64-71
: LGTM! Consistent button styling.The secondary type aligns with the new subtle design approach.
123-134
: LGTM! Consistent implementation.The changes mirror the account search result item, maintaining consistency.
apps/admin-x-activitypub/src/api/activitypub.ts (2)
72-72
: LGTM! Type enhancement.The isFollowing property in FollowAccount type improves type safety.
198-203
: LGTM! Clean unfollow implementation.The unfollow method follows the same pattern as other API methods, maintaining consistency.
apps/admin-x-activitypub/src/components/modals/ViewProfileModal.tsx (3)
101-101
: LGTM! Consistent button styling.The secondary type in the actor list aligns with the new subtle design.
329-332
: LGTM! Appropriate primary styling.Using primary type for the main profile button provides good visual hierarchy.
351-356
: LGTM! Clean button styling.The simplified expand/collapse button styling maintains focus on the main content.
apps/admin-x-activitypub/src/components/Profile.tsx (1)
215-220
: LGTM! Consistent implementation of FollowButton.The FollowButton is well-integrated into both Following and Followers tabs with consistent configuration and proper alignment.
Also applies to: 263-268
apps/admin-x-activitypub/src/hooks/useActivityPubQueries.ts (2)
7-8
: LGTM! Required imports added.The Actor and FollowAccount types are correctly imported to support the unfollow functionality.
279-279
: LGTM! Consistent query invalidation.The addition of
follows:index:following
query invalidation in useFollow maintains consistency with the new unfollow functionality.
ref https://linear.app/ghost/issue/AP-311/ui-to-unfollow-an-actor