diff --git a/KNOWLEDGE_BASE.md b/KNOWLEDGE_BASE.md index c79f78a31..06e99de75 100644 --- a/KNOWLEDGE_BASE.md +++ b/KNOWLEDGE_BASE.md @@ -523,6 +523,7 @@ Press **?** to open the shortcut reference. Navigation shortcuts work outside te | `y` | Copy the focused commit's link. | | `r` / `q` / `o` | Reply, quote, or show/hide replies. | | `s` | Star or unstar the focused commit. | +| `f` | Follow or unfollow the focused commit's author. | | `c` | Write a commit. | | `⌘/Ctrl+K` or `/` | Search developers and commits. | | `⌘/Ctrl+Enter` | Submit from the composer. | diff --git a/resources/js/components/commit-author-follow-button.tsx b/resources/js/components/commit-author-follow-button.tsx new file mode 100644 index 000000000..6e7986471 --- /dev/null +++ b/resources/js/components/commit-author-follow-button.tsx @@ -0,0 +1,78 @@ +import { usePage } from '@inertiajs/react'; +import { useRef } from 'react'; +import { + followVisitOptions, + resolveIsFollowing, + toggleFollow, + useIsFollowing, +} from '@/components/follow-button'; +import { signIn } from '@/lib/auth'; +import type { Commit, CursorPaginated, User } from '@/types'; + +/** + * Keyboard target for `f` on a focused commit: follow or unfollow that commit's author. + */ +export function CommitAuthorFollowButton({ + user, + keyShortcut, +}: { + user: Pick; + keyShortcut?: string; +}) { + const { + component, + props: { auth, log, commits }, + } = usePage<{ commits?: CursorPaginated }>(); + const [, setIsFollowing] = useIsFollowing(user, false); + const isBusy = useRef(false); + + if (auth.user?.id === user.id) { + return null; + } + + const onClick = () => { + if (auth.user === null) { + signIn(); + + return; + } + + if (isBusy.current) { + return; + } + + isBusy.current = true; + + void resolveIsFollowing(user).then((isFollowing) => { + if (isFollowing === null) { + isBusy.current = false; + + return; + } + + toggleFollow( + user, + isFollowing, + setIsFollowing, + () => { + isBusy.current = false; + }, + followVisitOptions(component, log, commits), + ); + }); + }; + + return ( + + ); +} diff --git a/resources/js/components/commit-card.tsx b/resources/js/components/commit-card.tsx index c6f4eda85..7ffeed88b 100644 --- a/resources/js/components/commit-card.tsx +++ b/resources/js/components/commit-card.tsx @@ -4,6 +4,7 @@ import type { MouseEvent, PointerEvent, ReactNode } from 'react'; import { useCallback, useContext, useRef, useState } from 'react'; import CommitController from '@/actions/App/Http/Controllers/CommitController'; import CommitLinkPreviewController from '@/actions/App/Http/Controllers/CommitLinkPreviewController'; +import { CommitAuthorFollowButton } from '@/components/commit-author-follow-button'; import { FoldedCommitBody } from '@/components/commit-body'; import { CommitImages } from '@/components/commit-images'; import { CommitPastelButton } from '@/components/commit-pastel-button'; @@ -277,6 +278,12 @@ export function CommitCard({ } onPointerLeave={onHover && ((event) => onHover(false, event))} > + {!wasDeleted && commit.author.type === 'user' && ( + + )}
, +): Promise { + const known = peekSharedState(isFollowingKey(user)); + + if (known !== undefined) { + return known; + } + + try { + const response = await fetch(userPreview(user).url, { + headers: { + Accept: 'application/json', + 'X-Requested-With': 'XMLHttpRequest', + }, + }); + + // Another follow control may have confirmed state while this preview was in flight. + const writtenWhilePending = peekSharedState( + isFollowingKey(user), + ); + + if (writtenWhilePending !== undefined) { + return writtenWhilePending; + } + + if (!response.ok) { + if (!toastWhenThrottled(response)) { + toast.error(`Could not check @${user.handle}. Try again.`); + } + + return null; + } + + const data = (await response.json()) as { + isFollowing: boolean; + isUnavailable: boolean; + }; + + // A follow may have landed while response.json() was still pending. + const writtenWhileParsing = peekSharedState( + isFollowingKey(user), + ); + + if (writtenWhileParsing !== undefined) { + return writtenWhileParsing; + } + + if (data.isUnavailable) { + rememberBlock(user); + + return null; + } + + refreshSharedState(new Map([[isFollowingKey(user), data.isFollowing]])); + + return data.isFollowing; + } catch { + toast.error(`Could not check @${user.handle}. Try again.`); + + return null; + } +} + +/** + * Follows or unfollows after resolving the current state, keeping every follow control in sync. + */ +export function toggleFollow( + user: Pick, + isFollowing: boolean, + setIsFollowing: (isFollowing: boolean) => void, + onFinish?: () => void, + visitOptions: Pick = {}, +): void { + const next = !isFollowing; + + if (peekSharedState(isPendingKey(user)) === true) { + onFinish?.(); + + return; + } + + refreshSharedState(new Map([[isPendingKey(user), true]])); + + router.visit( + next + ? UserFollowController.store(user) + : UserFollowController.destroy(user), + { + preserveScroll: true, + preserveState: true, + async: true, + ...visitOptions, + onSuccess: () => { + setIsFollowing(next); + router.flushAll(); + toast( + + {next + ? `Following @${user.handle}` + : `Unfollowed @${user.handle}`} + , + ); + }, + onHttpException: (response) => + onFollowHttpException(user, response), + onFinish: () => { + refreshSharedState(new Map([[isPendingKey(user), false]])); + onFinish?.(); + }, + }, + ); +} + export function useFollow( user: Pick, initialIsFollowing: boolean, diff --git a/resources/js/components/keyboard-shortcuts-dialog.tsx b/resources/js/components/keyboard-shortcuts-dialog.tsx index c4fb9ec01..71504bd2b 100644 --- a/resources/js/components/keyboard-shortcuts-dialog.tsx +++ b/resources/js/components/keyboard-shortcuts-dialog.tsx @@ -27,6 +27,10 @@ const groups = ( { keys: [['r']], label: 'Reply' }, { keys: [['q']], label: 'Quote' }, { keys: [['s']], label: 'Star or unstar' }, + { + keys: [['f']], + label: "Follow or unfollow the focused commit's author", + }, { keys: [['o']], label: 'Show or hide replies' }, { keys: [['y']], label: 'Copy link' }, ], diff --git a/resources/js/hooks/use-shared-state.ts b/resources/js/hooks/use-shared-state.ts index 6e9d51fe0..e44c72bf2 100644 --- a/resources/js/hooks/use-shared-state.ts +++ b/resources/js/hooks/use-shared-state.ts @@ -30,6 +30,15 @@ export function refreshSharedState(fresh: Map): void { } } +/** + * Reads a value only when something has written it, so callers can tell "unknown" from the initial default. + */ +export function peekSharedState( + key: string, +): T | undefined { + return values.has(key) ? (values.get(key) as T) : undefined; +} + export function useSharedState( key: string, initialValue: T, diff --git a/tests/Unit/CommitAuthorFollow.test.tsx b/tests/Unit/CommitAuthorFollow.test.tsx new file mode 100644 index 000000000..ecbe63a99 --- /dev/null +++ b/tests/Unit/CommitAuthorFollow.test.tsx @@ -0,0 +1,348 @@ +/// +import assert from 'node:assert/strict'; +import * as React from 'react'; +import type { ReactElement, ReactNode } from 'react'; +import { afterAll, test, vi } from 'vite-plus/test'; +import type { User } from '../../resources/js/types'; + +type Props = Record & { children?: ReactNode }; + +type VisitOptions = { + async?: boolean; + headers?: Record; + onHttpException?: (response: { status: number }) => boolean | void; + onSuccess?: () => void; + onFinish?: () => void; +}; + +const visits: (VisitOptions & { method?: string })[] = []; +const toasts: string[] = []; +const locations: string[] = []; +let flushAllCount = 0; +let authUser: { id: string } | null = { id: 'viewer' }; +let pageComponent = 'profile/show'; +let preview: { + isFollowing: boolean; + isUnavailable?: boolean; +} | null = { isFollowing: false }; +let fetchCount = 0; +let holdFetch = false; +let releaseFetch: (() => void) | null = null; +let holdJson = false; +let releaseJson: (() => void) | null = null; + +vi.doMock('react', () => ({ + ...React, + useEffect: () => {}, + useLayoutEffect: () => {}, + useRef: (initial: unknown) => ({ current: initial }), + useState: (initial: unknown) => [ + typeof initial === 'function' ? initial() : initial, + () => {}, + ], + useSyncExternalStore: (_subscribe: unknown, read: () => unknown) => read(), +})); + +vi.doMock('@inertiajs/react', async () => ({ + ...(await vi.importActual('@inertiajs/react')), + usePage: () => ({ + component: pageComponent, + props: { auth: { user: authUser }, log: null }, + }), + router: { + flushAll: () => { + flushAllCount += 1; + }, + visit: (url: { method: string }, options: VisitOptions) => + visits.push({ ...options, method: url.method }), + }, +})); + +vi.doMock('sonner', () => { + const toast = Object.assign( + (message: unknown) => { + toasts.push(typeof message === 'string' ? message : 'followed'); + }, + { + error: (message: string) => toasts.push(message), + success: (message: string) => toasts.push(message), + }, + ); + + return { toast }; +}); + +const { CommitAuthorFollowButton } = + await import('../../resources/js/components/commit-author-follow-button'); +const { FollowButton, resolveIsFollowing, toggleFollow } = + await import('../../resources/js/components/follow-button'); +const { peekSharedState, refreshSharedState } = + await import('../../resources/js/hooks/use-shared-state'); + +vi.stubGlobal('window', { + matchMedia: () => ({ matches: false }), + location: { + assign: (url: string) => locations.push(url), + }, +}); +vi.stubGlobal('document', { body: {} }); +vi.stubGlobal('fetch', async () => { + fetchCount += 1; + + if (holdFetch) { + await new Promise((resolve) => { + releaseFetch = resolve; + }); + } + + if (preview === null) { + return { + ok: false, + status: 500, + json: async () => ({}), + }; + } + + return { + ok: true, + status: 200, + json: async () => { + if (holdJson) { + await new Promise((resolve) => { + releaseJson = resolve; + }); + } + + return { + isUnavailable: false, + ...preview, + }; + }, + }; +}); +afterAll(() => vi.unstubAllGlobals()); + +function find( + node: ReactNode, + matches: (props: Props) => boolean, +): ReactElement | null { + if (Array.isArray(node)) { + for (const child of node) { + const found = find(child, matches); + + if (found) { + return found; + } + } + + return null; + } + + if (!React.isValidElement(node)) { + return null; + } + + return matches(node.props) ? node : find(node.props.children, matches); +} + +function author(handle: string): Pick { + return { id: handle, handle }; +} + +function followButton( + user: Pick, +): ReactElement | null { + const node = CommitAuthorFollowButton({ + user, + keyShortcut: 'f', + }) as ReactNode; + + if (node === null) { + return null; + } + + return find( + node, + (props) => props['data-test'] === 'commit-follow-author-button', + ); +} + +async function pressFollow(user: Pick): Promise { + const button = followButton(user); + assert.ok(button); + (button.props.onClick as () => void)(); + await new Promise((resolve) => setTimeout(resolve)); +} + +function followFromProfile(user: Pick): void { + const profileButton = find( + FollowButton({ user, isFollowing: false }), + (props) => props['data-test'] === 'follow-button', + ); + assert.ok(profileButton); + + (profileButton.props.onStart as () => void)(); + (profileButton.props.onSuccess as () => void)(); + (profileButton.props.onFinish as () => void)(); +} + +test('the focused commit exposes f as its follow shortcut', () => { + authUser = { id: 'viewer' }; + const button = followButton(author('taylor')); + + assert.ok(button); + assert.equal(button.props['aria-keyshortcuts'], 'f'); + assert.equal(button.props['aria-label'], 'Follow or unfollow @taylor'); +}); + +test('your own commit has no follow shortcut', () => { + authUser = { id: 'taylor' }; + + assert.equal(followButton(author('taylor')), null); +}); + +test('guests are sent to sign in instead of following', async () => { + authUser = null; + locations.length = 0; + visits.length = 0; + + await pressFollow(author('guest-target')); + + assert.equal(visits.length, 0); + assert.ok(locations.at(-1)?.includes('login')); +}); + +test('f follows an author who is not already followed', async () => { + authUser = { id: 'viewer' }; + preview = { isFollowing: false }; + fetchCount = 0; + visits.length = 0; + + await pressFollow(author('newfriend')); + + assert.equal(fetchCount, 1); + assert.equal(visits.at(-1)?.method, 'post'); + assert.equal(visits.at(-1)?.async, true); + assert.equal(visits.at(-1)?.headers?.['X-Follow-Request'], 'true'); + visits.at(-1)?.onSuccess?.(); + visits.at(-1)?.onFinish?.(); +}); + +test('f unfollows once shared state already knows they are followed', async () => { + authUser = { id: 'viewer' }; + const user = author('known'); + followFromProfile(user); + fetchCount = 0; + visits.length = 0; + + await pressFollow(user); + + assert.equal(fetchCount, 0); + assert.equal(visits.at(-1)?.method, 'delete'); +}); + +test('resolveIsFollowing remembers a preview result for later shortcuts', async () => { + authUser = { id: 'viewer' }; + preview = { isFollowing: true }; + fetchCount = 0; + + assert.equal(await resolveIsFollowing(author('previewed')), true); + assert.equal(await resolveIsFollowing(author('previewed')), true); + assert.equal(fetchCount, 1); +}); + +test('resolveIsFollowing gives up when the author is unavailable', async () => { + authUser = { id: 'viewer' }; + preview = { isFollowing: false, isUnavailable: true }; + + assert.equal(await resolveIsFollowing(author('blocked-author')), null); +}); + +test('resolveIsFollowing toasts when the preview fails for a reason other than throttling', async () => { + authUser = { id: 'viewer' }; + preview = null; + toasts.length = 0; + + assert.equal(await resolveIsFollowing(author('down')), null); + assert.equal(toasts.at(-1), 'Could not check @down. Try again.'); +}); + +test('a follow that lands while the preview is pending wins over a stale response', async () => { + authUser = { id: 'viewer' }; + const user = author('raced'); + preview = { isFollowing: false }; + holdFetch = true; + + const pending = resolveIsFollowing(user); + await new Promise((resolve) => setTimeout(resolve)); + const resume = releaseFetch; + + assert.equal(typeof resume, 'function'); + + followFromProfile(user); + resume?.(); + holdFetch = false; + + assert.equal(await pending, true); +}); + +test('a follow that lands while the preview body is parsing wins over a stale response', async () => { + authUser = { id: 'viewer' }; + const user = author('json-raced'); + preview = { isFollowing: false }; + holdJson = true; + + const pending = resolveIsFollowing(user); + await new Promise((resolve) => setTimeout(resolve)); + const resume = releaseJson; + + assert.equal(typeof resume, 'function'); + + followFromProfile(user); + resume?.(); + holdJson = false; + + assert.equal(await pending, true); +}); + +test('toggleFollow posts a follow and reports success', () => { + visits.length = 0; + toasts.length = 0; + flushAllCount = 0; + let following = false; + + toggleFollow(author('toastable'), false, (next) => { + following = next; + }); + + assert.equal(visits.at(-1)?.method, 'post'); + assert.equal(peekSharedState('follow_pending:toastable'), true); + visits.at(-1)?.onSuccess?.(); + visits.at(-1)?.onFinish?.(); + assert.equal(following, true); + assert.equal(toasts.at(-1), 'followed'); + assert.equal(flushAllCount, 1); + assert.equal(peekSharedState('follow_pending:toastable'), false); +}); + +test('toggleFollow skips while another follow for that author is pending', () => { + visits.length = 0; + let finished = false; + const user = author('busy-author'); + + refreshSharedState(new Map([[`follow_pending:${user.id}`, true]])); + + toggleFollow( + user, + false, + () => {}, + () => { + finished = true; + }, + ); + + assert.equal(visits.length, 0); + assert.equal(finished, true); + + refreshSharedState(new Map([[`follow_pending:${user.id}`, false]])); +}); diff --git a/tests/Unit/CommitStarButton.test.tsx b/tests/Unit/CommitStarButton.test.tsx index 55796d0c4..b46c51158 100644 --- a/tests/Unit/CommitStarButton.test.tsx +++ b/tests/Unit/CommitStarButton.test.tsx @@ -11,6 +11,7 @@ import type { Commit } from '../../resources/js/types'; async function render( user: { id: string } | null, commit: Partial = {}, + keyShortcut?: string, ): Promise { const result = await createInertiaApp<{ auth: { organizations: [], user: { id: string } | null }; @@ -38,6 +39,7 @@ async function render( ...commit, } as Commit } + keyShortcut={keyShortcut} /> ), render: renderToString, @@ -54,7 +56,7 @@ async function render( } for (const status of ['approved', 'pending', 'rejected'] as const) { - test(`authors see their ${status} commit's star count and may open who starred`, async () => { + void test(`authors see their ${status} commit's star count and may open who starred`, async () => { const body = await render({ id: 'author' }, { status }); assert.match(body, /data-test="own-star-count"/); @@ -65,7 +67,7 @@ for (const status of ['approved', 'pending', 'rejected'] as const) { }); } - test('authors see an accessible empty star count', async () => { +void test('authors see an accessible empty star count', async () => { const body = await render({ id: 'author' }, { stars_count: 0 }); assert.match(body, /aria-label="No stars yet"/); @@ -77,7 +79,7 @@ for (const status of ['approved', 'pending', 'rejected'] as const) { }); for (const user of [null, { id: 'viewer' }]) { - test(`approved commits keep the star control for ${user === null ? 'guests' : 'other readers'}`, async () => { + void test(`approved commits keep the star control for ${user === null ? 'guests' : 'other readers'}`, async () => { const body = await render(user); assert.match(body, /data-test="star-commit-button"/); @@ -87,7 +89,14 @@ for (const user of [null, { id: 'viewer' }]) { }); } - test('unstarred commits show only the star in the star control', async () => { +void test('star button exposes the keyboard shortcut used by j/k navigation', async () => { + const body = await render({ id: 'viewer' }, {}, 's'); + + assert.match(body, /data-test="star-commit-button"/); + assert.match(body, /aria-keyshortcuts="s"/); +}); + +void test('unstarred commits show only the star in the star control', async () => { const body = await render({ id: 'viewer' }, { stars_count: 0 }); assert.match( @@ -96,7 +105,7 @@ for (const user of [null, { id: 'viewer' }]) { ); }); - test('starred commits keep the count slot inside the star control', async () => { +void test('starred commits keep the count slot inside the star control', async () => { const body = await render({ id: 'viewer' }, { stars_count: 1 }); assert.doesNotMatch(body, /mr-\[calc/); @@ -104,7 +113,7 @@ for (const user of [null, { id: 'viewer' }]) { }); for (const status of ['pending', 'rejected'] as const) { - test(`other readers cannot star a ${status} commit`, async () => { + void test(`other readers cannot star a ${status} commit`, async () => { const body = await render({ id: 'viewer' }, { status }); assert.doesNotMatch( @@ -115,7 +124,7 @@ for (const status of ['pending', 'rejected'] as const) { } for (const status of ['approved', 'pending', 'rejected'] as const) { - test(`authors see their ${status} reply's star count and may open who starred`, async () => { + void test(`authors see their ${status} reply's star count and may open who starred`, async () => { const body = await render( { id: 'author' }, { status, parent_id: 'parent' }, diff --git a/tests/Unit/NotificationIndex.test.tsx b/tests/Unit/NotificationIndex.test.tsx index ad6f1db8d..083544b04 100644 --- a/tests/Unit/NotificationIndex.test.tsx +++ b/tests/Unit/NotificationIndex.test.tsx @@ -207,7 +207,7 @@ test('shows the image of a starred image-only commit beside who starred it', asy commit: { id: 'image-only', body: '', - body_segments: [], + body_segments: [] as Commit['body_segments'], images: [ { id: 'image',