diff --git a/src/components/MediaPreview.tsx b/src/components/MediaPreview.tsx index de07a03b23..89ee1c83f2 100644 --- a/src/components/MediaPreview.tsx +++ b/src/components/MediaPreview.tsx @@ -53,31 +53,32 @@ export function Embed({ ) } else if (e.type === 'gallery') { // Notification/DM preview is a narrow inline strip; cap at 4 tiles so - // a 10-image gallery doesn't blow out the row width. - return ( - - {e.view.items - .filter(AppBskyEmbedGallery.isViewImage) - .slice(0, 4) - .map(item => { - const image: AppBskyEmbedImages.ViewImage = { - thumb: item.thumbnail, - fullsize: item.fullsize, - alt: item.alt, - aspectRatio: item.aspectRatio, - } - return peekable ? ( - - ) : ( - - ) - })} - - ) + // a 10-image gallery doesn't blow out the row width. Single pass instead + // of filter().slice().map() so we stop at 4 viewable items rather than + // walking every item in a 10-image gallery. + const tiles: React.ReactNode[] = [] + for (const item of e.view.items) { + if (tiles.length >= 4) break + if (!AppBskyEmbedGallery.isViewImage(item)) continue + if (peekable) { + const image: AppBskyEmbedImages.ViewImage = { + thumb: item.thumbnail, + fullsize: item.fullsize, + alt: item.alt, + aspectRatio: item.aspectRatio, + } + tiles.push() + } else { + tiles.push( + , + ) + } + } + return {tiles} } else if (e.type === 'link') { if (!e.view.external.thumb) return null if (!isGifEmbed(e.view.external.uri)) return null diff --git a/src/components/dms/MessageOverlays.tsx b/src/components/dms/MessageOverlays.tsx index 7849147104..41228f2899 100644 --- a/src/components/dms/MessageOverlays.tsx +++ b/src/components/dms/MessageOverlays.tsx @@ -4,7 +4,6 @@ import { useContext, useEffect, useMemo, - useRef, useState, } from 'react' import {LayoutAnimation} from 'react-native' @@ -60,28 +59,8 @@ export function MessageOverlays({children}: {children: React.ReactNode}) { } | null>(null) const [afterReportTarget, setAfterReportTarget] = useState(null) - const [reactionsTargetId, setReactionsTargetId] = useState( - null, - ) - const [lastKnownReactionsMessage, setLastKnownReactionsMessage] = + const [reactionsTarget, setReactionsTarget] = useState(null) - const reactionsOpenRequestedFor = useRef(null) - - const liveReactionsMessage = useMemo(() => { - if (!reactionsTargetId) return null - for (const item of convo.items) { - if ( - (item.type === 'message' || item.type === 'pending-message') && - item.message.id === reactionsTargetId - ) { - return item.message - } - } - return null - }, [convo.items, reactionsTargetId]) - - const displayReactionsMessage = - liveReactionsMessage ?? lastKnownReactionsMessage const openDeleteMessage = useCallback( (message: ChatBskyConvoDefs.MessageView) => { @@ -104,42 +83,19 @@ export function MessageOverlays({children}: {children: React.ReactNode}) { const openReactions = useCallback( (message: ChatBskyConvoDefs.MessageView) => { - reactionsOpenRequestedFor.current = message.id - setReactionsTargetId(message.id) + setReactionsTarget(message) }, [], ) - // The dialog is conditionally mounted, so we can't open it in the same tick - // that we set the target - the control ref isn't attached yet. Open in an - // effect after the dialog has mounted with a live message resolved. + // These dialogs are conditionally mounted, so we can't open them in the same + // tick that we set their targets - the control refs aren't attached yet. Open + // in an effect after the dialog has mounted. useEffect(() => { - if ( - liveReactionsMessage && - reactionsOpenRequestedFor.current === liveReactionsMessage.id - ) { - reactionsOpenRequestedFor.current = null + if (reactionsTarget) { reactionsControl.open() } - }, [liveReactionsMessage, reactionsControl]) - - // Keep a snapshot of the live message so the dialog can finish its close - // animation if the underlying message disappears from convo.items. - useEffect(() => { - if (liveReactionsMessage) { - // eslint-disable-next-line react-hooks/set-state-in-effect - setLastKnownReactionsMessage(liveReactionsMessage) - } - }, [liveReactionsMessage]) - - useEffect(() => { - if (reactionsTargetId && !liveReactionsMessage) { - reactionsControl.close() - // eslint-disable-next-line react-hooks/set-state-in-effect - setReactionsTargetId(null) - reactionsOpenRequestedFor.current = null - } - }, [reactionsTargetId, liveReactionsMessage, reactionsControl]) + }, [reactionsTarget, reactionsControl]) useEffect(() => { if (afterReportTarget) { @@ -169,18 +125,13 @@ export function MessageOverlays({children}: {children: React.ReactNode}) { [openDeleteMessage, openReportMessage, openReactions], ) - const convoId = convo.convo.view.id - const reportSubject = useMemo( - () => - reportTarget - ? ({ - view: 'message', - convoId, - message: reportTarget.message, - } as const) - : undefined, - [reportTarget, convoId], - ) + const reportSubject = reportTarget + ? ({ + view: 'message', + convoId: convo.convo.view.id, + message: reportTarget.message, + } as const) + : undefined return ( @@ -202,16 +153,12 @@ export function MessageOverlays({children}: {children: React.ReactNode}) { onClose={() => setAfterReportTarget(null)} /> )} - {displayReactionsMessage && ( + {reactionsTarget && ( { - setReactionsTargetId(null) - setLastKnownReactionsMessage(null) - reactionsOpenRequestedFor.current = null - }} + message={reactionsTarget} + onClose={() => setReactionsTarget(null)} /> )} void, +) { + const {t: l} = useLingui() + return useCallback( + (next: ComposerImage[]) => { + const result = applyGalleryCap(currentCount, next) + if (result.status === 'full') { + Toast.show( + l({ + message: `You can only add up to ${MAX_GALLERY_IMAGES} images per post`, + comment: + 'Toast shown when the user tries to add more images but the post gallery is already at the cap', + }), + {type: 'warning'}, + ) + return + } + if (result.status === 'partial') { + Toast.show( + l({ + message: `Only ${result.accepted.length} of ${next.length} ${plural(next.length, {one: 'image', other: 'images'})} added; limit is ${MAX_GALLERY_IMAGES}`, + comment: + 'Toast shown when adding images would exceed the post gallery cap; only the first N are kept', + }), + {type: 'warning'}, + ) + } + dispatchPostAction({ + type: 'embed_add_images', + images: result.accepted, + }) + }, + [currentCount, dispatchPostAction, l], + ) +} + type Props = ComposerOpts export const ComposePost = ({ replyTo, @@ -1426,42 +1464,11 @@ let ComposerPost = memo(function ComposerPost({ [dispatch, post.id], ) - const onImageAdd = useCallback( - (next: ComposerImage[]) => { - const media = post.embed.media - const currentCount = - media?.type === 'images' || media?.type === 'gallery' - ? media.images.length - : 0 - const result = applyGalleryCap(currentCount, next) - if (result.status === 'full') { - Toast.show( - l({ - message: `You can only add up to ${MAX_GALLERY_IMAGES} images per post`, - comment: - 'Toast shown when the user tries to add more images but the post gallery is already at the cap', - }), - {type: 'warning'}, - ) - return - } - if (result.status === 'partial') { - Toast.show( - l({ - message: `Only ${result.accepted.length} of ${next.length} ${plural(next.length, {one: 'image', other: 'images'})} added; limit is ${MAX_GALLERY_IMAGES}`, - comment: - 'Toast shown when adding images would exceed the post gallery cap; only the first N are kept', - }), - {type: 'warning'}, - ) - } - dispatchPost({ - type: 'embed_add_images', - images: result.accepted, - }) - }, - [dispatchPost, l, post.embed.media], - ) + const postImagesCount = + post.embed.media?.type === 'images' || post.embed.media?.type === 'gallery' + ? post.embed.media.images.length + : 0 + const onImageAdd = useAddImagesWithCap(postImagesCount, dispatchPost) const onNewLink = useCallback( (uri: string) => { @@ -1989,37 +1996,7 @@ function ComposerFooter({ isMediaSelectionDisabled = !!media } - const onImageAdd = useCallback( - (next: ComposerImage[]) => { - const result = applyGalleryCap(images.length, next) - if (result.status === 'full') { - Toast.show( - l({ - message: `You can only add up to ${MAX_GALLERY_IMAGES} images per post`, - comment: - 'Toast shown when the user tries to add more images but the post gallery is already at the cap', - }), - {type: 'warning'}, - ) - return - } - if (result.status === 'partial') { - Toast.show( - l({ - message: `Only ${result.accepted.length} of ${next.length} ${plural(next.length, {one: 'image', other: 'images'})} added; limit is ${MAX_GALLERY_IMAGES}`, - comment: - 'Toast shown when adding images would exceed the post gallery cap; only the first N are kept', - }), - {type: 'warning'}, - ) - } - dispatch({ - type: 'embed_add_images', - images: result.accepted, - }) - }, - [dispatch, images.length, l], - ) + const onImageAdd = useAddImagesWithCap(images.length, dispatch) const onSelectGif = useCallback( (gif: Gif) => { diff --git a/src/view/com/composer/drafts/state/api.ts b/src/view/com/composer/drafts/state/api.ts index a6aab1425a..817820effb 100644 --- a/src/view/com/composer/drafts/state/api.ts +++ b/src/view/com/composer/drafts/state/api.ts @@ -15,7 +15,7 @@ import {createPublicAgent} from '#/state/session/agent' import { type ComposerState, type EmbedDraft, - MAX_IMAGES, + LEGACY_IMAGES_EMBED_MAX, type PostDraft, } from '#/view/com/composer/state/composer' import {type VideoState} from '#/view/com/composer/state/video' @@ -535,7 +535,7 @@ export async function draftToComposerPosts( } if (restoredImages.length > 0) { embed.media = - restoredImages.length <= MAX_IMAGES + restoredImages.length <= LEGACY_IMAGES_EMBED_MAX ? {type: 'images', images: restoredImages} : {type: 'gallery', images: restoredImages} } diff --git a/src/view/com/composer/state/composer.ts b/src/view/com/composer/state/composer.ts index c698892863..35e1706007 100644 --- a/src/view/com/composer/state/composer.ts +++ b/src/view/com/composer/state/composer.ts @@ -160,7 +160,14 @@ export type ComposerAction = draftId: string } -export const MAX_IMAGES = 4 +/** + * Threshold for picking between embed variants. <= this count uses the + * legacy `app.bsky.embed.images` shape; > this count promotes to + * `app.bsky.embed.gallery`. Named to flag that if/when we deprecate the + * legacy images embed entirely, this constant (and the variant split it + * gates) should go away. + */ +export const LEGACY_IMAGES_EMBED_MAX = 4 export const MAX_GALLERY_IMAGES = 10 /** @@ -174,8 +181,8 @@ export const MAX_GALLERY_IMAGES = 10 function imagesToMediaVariant( images: ComposerImage[], ): ImagesMedia | GalleryMedia { - return images.length <= MAX_IMAGES - ? {type: 'images', images: images.slice(0, MAX_IMAGES)} + return images.length <= LEGACY_IMAGES_EMBED_MAX + ? {type: 'images', images: images.slice(0, LEGACY_IMAGES_EMBED_MAX)} : {type: 'gallery', images: images.slice(0, MAX_GALLERY_IMAGES)} } @@ -366,8 +373,8 @@ function postReducer(state: PostDraft, action: PostAction): PostDraft { : 0 const incomingCount = prevCount + action.images.length if (incomingCount > MAX_GALLERY_IMAGES) { - // TODO: surface this to the user via a toast once the composer - // state shape supports reducer-emitted errors. The hard slice in + // Defense in depth: callers (applyGalleryCap in Composer) should have + // already trimmed and surfaced a toast. The hard slice in // imagesToMediaVariant still drops the excess so the cap holds. logger.warn('composer: image add exceeds MAX_GALLERY_IMAGES', { prevCount,