From 7d72dfb1cb514a9ab8ee2874390c667d49a78e8b Mon Sep 17 00:00:00 2001 From: Samuel Newman Date: Tue, 7 May 2024 20:05:40 +0100 Subject: [PATCH] [GIFs] Restore default alt text (#3893) * restore default alt text * factor out gif alt logic + enable require alt text setting * rm console.log * don't prefill input + esc handling * typo * Nits * shorten user alt prefix * Remove unnecessary condition, rename for clarity * Add comment --------- Co-authored-by: Dan Abramov --- src/lib/gif-alt-text.ts | 36 +++++++++++++++++++ src/view/com/composer/Composer.tsx | 40 +++++++++++++++------- src/view/com/composer/GifAltText.tsx | 16 ++++++--- src/view/com/util/post-embeds/GifEmbed.tsx | 12 ++++--- 4 files changed, 84 insertions(+), 20 deletions(-) create mode 100644 src/lib/gif-alt-text.ts diff --git a/src/lib/gif-alt-text.ts b/src/lib/gif-alt-text.ts new file mode 100644 index 00000000..41738c50 --- /dev/null +++ b/src/lib/gif-alt-text.ts @@ -0,0 +1,36 @@ +// Kind of a hack. We needed some way to distinguish these. +const USER_ALT_PREFIX = 'Alt: ' +const DEFAULT_ALT_PREFIX = 'ALT: ' + +export function createGIFDescription( + tenorDescription: string, + preferredAlt: string = '', +) { + preferredAlt = preferredAlt.trim() + if (preferredAlt !== '') { + return USER_ALT_PREFIX + preferredAlt + } else { + return DEFAULT_ALT_PREFIX + tenorDescription + } +} + +export function parseAltFromGIFDescription(description: string): { + isPreferred: boolean + alt: string +} { + if (description.startsWith(USER_ALT_PREFIX)) { + return { + isPreferred: true, + alt: description.replace(USER_ALT_PREFIX, ''), + } + } else if (description.startsWith(DEFAULT_ALT_PREFIX)) { + return { + isPreferred: false, + alt: description.replace(DEFAULT_ALT_PREFIX, ''), + } + } + return { + isPreferred: false, + alt: description, + } +} diff --git a/src/view/com/composer/Composer.tsx b/src/view/com/composer/Composer.tsx index f472bb2e..61c33902 100644 --- a/src/view/com/composer/Composer.tsx +++ b/src/view/com/composer/Composer.tsx @@ -18,6 +18,10 @@ import {msg, Trans} from '@lingui/macro' import {useLingui} from '@lingui/react' import {observer} from 'mobx-react-lite' +import { + createGIFDescription, + parseAltFromGIFDescription, +} from '#/lib/gif-alt-text' import {LikelyType} from '#/lib/link-meta/link-meta' import {logEvent} from '#/lib/statsig/statsig' import {logger} from '#/logger' @@ -211,11 +215,25 @@ export const ComposePost = observer(function ComposePost({ [gallery, track], ) + const isAltTextRequiredAndMissing = useMemo(() => { + if (!requireAltTextEnabled) return false + + if (gallery.needsAltText) return true + if (extGif) { + if (!extLink?.meta?.description) return true + + const parsedAlt = parseAltFromGIFDescription(extLink.meta.description) + if (!parsedAlt.isPreferred) return true + } + return false + }, [gallery.needsAltText, extLink, extGif, requireAltTextEnabled]) + const onPressPublish = async () => { if (isProcessing || graphemeLength > MAX_GRAPHEME_LENGTH) { return } - if (requireAltTextEnabled && gallery.needsAltText) { + + if (isAltTextRequiredAndMissing) { return } @@ -298,10 +316,8 @@ export const ComposePost = observer(function ComposePost({ } const canPost = useMemo( - () => - graphemeLength <= MAX_GRAPHEME_LENGTH && - (!requireAltTextEnabled || !gallery.needsAltText), - [graphemeLength, requireAltTextEnabled, gallery.needsAltText], + () => graphemeLength <= MAX_GRAPHEME_LENGTH && !isAltTextRequiredAndMissing, + [graphemeLength, isAltTextRequiredAndMissing], ) const selectTextInputPlaceholder = replyTo ? _(msg`Write your reply`) @@ -328,7 +344,7 @@ export const ComposePost = observer(function ComposePost({ image: gif.media_formats.preview.url, likelyType: LikelyType.HTML, title: gif.content_description, - description: '', + description: createGIFDescription(gif.content_description), }, }) setExtGif(gif) @@ -343,11 +359,11 @@ export const ComposePost = observer(function ComposePost({ ? { ...ext, meta: { - ...ext?.meta, - description: - altText.trim().length === 0 - ? '' - : `Alt text: ${altText.trim()}`, + ...ext.meta, + description: createGIFDescription( + ext.meta.title ?? '', + altText, + ), }, } : ext, @@ -433,7 +449,7 @@ export const ComposePost = observer(function ComposePost({ )} - {requireAltTextEnabled && gallery.needsAltText && ( + {isAltTextRequiredAndMissing && ( - {link.description ? ( + {parsedAlt.isPreferred ? ( ) : ( @@ -102,7 +104,7 @@ export function GifAltText({ onSubmit={onPressSubmit} link={link} params={params} - initalValue={link.description.replace('Alt text: ', '')} + initialValue={parsedAlt.isPreferred ? parsedAlt.alt : ''} key={link.uri} /> @@ -114,15 +116,16 @@ function AltTextInner({ onSubmit, link, params, - initalValue, + initialValue: initalValue, }: { onSubmit: (text: string) => void link: AppBskyEmbedExternal.ViewExternal params: EmbedPlayerParams - initalValue: string + initialValue: string }) { const {_} = useLingui() const [altText, setAltText] = useState(initalValue) + const control = Dialog.useDialogContext() const onPressSubmit = useCallback(() => { onSubmit(altText) @@ -147,6 +150,11 @@ function AltTextInner({ multiline numberOfLines={3} autoFocus + onKeyPress={({nativeEvent}) => { + if (nativeEvent.key === 'Escape') { + control.close() + } + }} /> diff --git a/src/view/com/util/post-embeds/GifEmbed.tsx b/src/view/com/util/post-embeds/GifEmbed.tsx index 286b5799..deb82655 100644 --- a/src/view/com/util/post-embeds/GifEmbed.tsx +++ b/src/view/com/util/post-embeds/GifEmbed.tsx @@ -6,6 +6,7 @@ import {msg, Trans} from '@lingui/macro' import {useLingui} from '@lingui/react' import {HITSLOP_10} from '#/lib/constants' +import {parseAltFromGIFDescription} from '#/lib/gif-alt-text' import {isWeb} from '#/platform/detection' import {EmbedPlayerParams} from 'lib/strings/embed-player' import {useAutoplayDisabled} from 'state/preferences' @@ -116,6 +117,11 @@ export function GifEmbed({ playerRef.current?.toggleAsync() }, []) + const parsedAlt = React.useMemo( + () => parseAltFromGIFDescription(link.description), + [link], + ) + return ( @@ -140,12 +146,10 @@ export function GifEmbed({ onPlayerStateChange={onPlayerStateChange} ref={playerRef} accessibilityHint={_(msg`Animated GIF`)} - accessibilityLabel={link.description.replace('Alt text: ', '')} + accessibilityLabel={parsedAlt.alt} /> - {!hideAlt && link.description.startsWith('Alt text: ') && ( - - )} + {!hideAlt && parsedAlt.isPreferred && } )