Added follow buttons to Reader and Notifications #25561
Conversation
WalkthroughThis PR bumps apps/activitypub package version 2.0.2 → 2.0.3, adds optional booleans Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/activitypub/src/components/global/FollowButton.tsx (1)
10-10: Remove unusedtypeprop from interface.The
typeprop is defined in the interface but never used in the component. Consider removing it to clean up the API surface.interface FollowButtonProps { className?: string; following: boolean; handle: string; - type?: 'primary' | 'secondary'; variant?: 'default' | 'link'; followsYou?: boolean; onFollow?: () => void; onUnfollow?: () => void; 'data-testid'?: string; }apps/activitypub/src/views/Notifications/Notifications.tsx (1)
385-385: Minor: Reorder Tailwind classes for consistency.The hover modifier
group-hover/item:underlinecomes before base classes. Consider reordering for conventional Tailwind class ordering (base classes first, then modifiers).- <span className='group-hover/item:underline ml-2 line-clamp-1 text-base font-semibold dark:text-white'>{actor.name}</span> + <span className='ml-2 line-clamp-1 text-base font-semibold dark:text-white group-hover/item:underline'>{actor.name}</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/activitypub/package.json(1 hunks)apps/activitypub/src/api/activitypub.ts(1 hunks)apps/activitypub/src/components/global/FollowButton.tsx(4 hunks)apps/activitypub/src/views/Inbox/components/Reader.tsx(2 hunks)apps/activitypub/src/views/Notifications/Notifications.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Use CVA (Class Variance Authority) for component variants when useful instead of heavy prop configuration
Applied to files:
apps/activitypub/src/components/global/FollowButton.tsx
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
apps/activitypub/package.json
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
apps/activitypub/package.json
📚 Learning: 2025-03-13T09:02:50.102Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the Ghost ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.
Applied to files:
apps/activitypub/package.json
🧬 Code graph analysis (1)
apps/activitypub/src/views/Notifications/Notifications.tsx (2)
apps/admin-x-framework/src/api/activitypub.ts (1)
ActorProperties(29-63)apps/activitypub/src/utils/handle-profile-click.ts (1)
handleProfileClick(5-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Setup
- GitHub Check: Build & Push
- GitHub Check: Setup
🔇 Additional comments (7)
apps/activitypub/package.json (1)
3-3: LGTM!Version bump to 2.0.3 appropriately reflects the new follow button functionality added in this PR.
apps/activitypub/src/api/activitypub.ts (1)
108-109: LGTM!The new optional
followedByMeandfollowsMeproperties on the notification actor align with the existingAccountinterface pattern (lines 23-24) and properly support the follow button functionality in notifications.apps/activitypub/src/views/Inbox/components/Reader.tsx (2)
811-834: LGTM!The header layout refactoring cleanly separates author information from the follow action. The conditional rendering logic correctly hides the follow button for content authored by the current user and during loading states. The nullish coalescing
actor.followedByMe ?? falsesafely handles potentially undefined values.
829-832: This code already has proper type safety.The
actorobject comes frommapPostToActivity(post)which returns a properly typedActivitywith anactorfield of typeActorProperties. ThefollowedByMeproperty is explicitly mapped frompost.author.followedByMeand is safely accessed using the nullish coalescing operator. No refactoring is required.apps/activitypub/src/components/global/FollowButton.tsx (1)
66-89: LGTM!Clean implementation of the link variant with proper conditional styling. The
buttonTextcomputation handles all three states (Following, Follow back, Follow) correctly. Event handlers appropriately prevent default and stop propagation to avoid triggering parent click handlers.apps/activitypub/src/views/Notifications/Notifications.tsx (2)
377-395: LGTM on multi-actor follow buttons.The per-actor follow buttons in the expanded group view are well-implemented. The conditional rendering correctly shows the button only for follow-type notifications where the actor hasn't been followed yet, and passes
followsYoufor the "Follow back" text.
420-428: LGTM on single-actor follow button.The selective application of follow buttons to
follow,reply, andmentionnotification types makes sense—these are person-centric interactions where discovering and following the actor is relevant, unlikelikeandrepostwhich are content-centric.
E2E Tests FailedTo view the Playwright test report locally, run: REPORT_DIR=$(mktemp -d) && gh run download 19816059648 -n playwright-report -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR" |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/activitypub/src/views/Notifications/Notifications.tsx (2)
333-365: Expanded multi-actor follow list: good interaction model, consider stricter follow-state checks
- The expanded “show all” area correctly:
- Isolates its own click handling (row + chevron button both stop propagation).
- Routes profile clicks through
handleProfileClick(actor.handle, navigate)without toggling the group.- Renders a per-actor FollowButton only for follow-type groups and when
!actor.followedByMe, which matches the “quick follow back” intent.One small robustness tweak you could consider (optional):
{group.type === 'follow' && actor.followedByMe === false && ( <FollowButton following={false} followsYou={actor.followsMe === true} handle={actor.handle} variant="link" /> )}Using explicit
=== false/truekeeps behavior predictable iffollowedByMeorfollowsMeare everundefined/ “unknown” rather than strictly boolean.Also applies to: 368-395
410-428: Single-actor header follow button: condition and layout look sensible
- The header now uses a two-column flex layout: description + (for single-actor) timestamp on the left, optional FollowButton on the right. This gives a clear, compact affordance without changing navigation behavior.
- The condition
group.actors.length === 1 && (group.type === 'follow' || group.type === 'reply' || group.type === 'mention') && !group.actors[0].followedByMematches the product goal of enabling “quick follow” from:
- direct follow notifications,
- replies, and
- mentions,
while avoiding redundant buttons when you already follow the actor.As with the multi-actor case, you might optionally tighten the predicate to
group.actors[0].followedByMe === falseand passfollowsYou={group.actors[0].followsMe === true}for clearer handling of undefined values, but the current logic is functionally acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/activitypub/src/components/global/FollowButton.tsx(4 hunks)apps/activitypub/src/views/Notifications/Notifications.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Use CVA (Class Variance Authority) for component variants when useful instead of heavy prop configuration
Applied to files:
apps/activitypub/src/components/global/FollowButton.tsx
🧬 Code graph analysis (1)
apps/activitypub/src/views/Notifications/Notifications.tsx (3)
apps/admin-x-framework/src/api/activitypub.ts (1)
ActorProperties(29-63)apps/activitypub/src/utils/handle-profile-click.ts (1)
handleProfileClick(5-12)apps/activitypub/src/utils/render-timestamp.tsx (1)
renderTimestamp(15-40)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: ActivityPub tests
- GitHub Check: Build & Push
🔇 Additional comments (2)
apps/activitypub/src/components/global/FollowButton.tsx (1)
11-12: Follow-back label and link variant wiring look consistent
buttonTextcorrectly prioritizesisFollowingoverfollowsYou, so you never see “Follow back” when already following.- Both the default and
linkvariants sharehandleClick, so optimistic local state and mutation behavior stay consistent.- Stopping propagation and preventing default in the
linkvariant matches the default button behavior, which avoids accidental navigation when used inside links/lists.No functional concerns from this change; the new props and variant behavior look coherent with the existing state model.
Also applies to: 24-26, 66-89, 106-107
apps/activitypub/src/views/Notifications/Notifications.tsx (1)
8-8: Profile preview alignment and FollowButton import are straightforwardAdding the FollowButton import and passing
align="center"intoProfilePreviewHoverCardare minimal, self-contained changes; they don’t alter notification logic and keep the hover-card usage consistent.Also applies to: 122-135
ref https://linear.app/ghost/issue/BER-2980/easier-follow-button-in-article-view
ref https://linear.app/ghost/issue/BER-2981/follow-button-in-notifications