Address PR review feedback for invite friends

- Remove commented-out Dialog.Handle in InviteFriendsDialog
- Defer NUX -> invite handoff with requestAnimationFrame instead of setTimeout
- Un-nest the dismiss Pressable in FollowersPromoBanner (avoids Android touch conflicts)
- Drop redundant toLowerCase in getInviteShareUrl (handles are API-normalized, matches makeProfileLink)
- Request write-only media library permission when saving the QR code
- Migrate InviteFriendsAnnouncement to the l macro
This commit is contained in:
vineyardbovines
2026-06-02 09:31:08 -04:00
parent bb9f0b9bbf
commit 218c997776
6 changed files with 58 additions and 54 deletions
@@ -1,9 +1,7 @@
import {useCallback} from 'react' import {useCallback} from 'react'
import {View} from 'react-native' import {View} from 'react-native'
import {Image} from 'expo-image' import {Image} from 'expo-image'
import {msg} from '@lingui/core/macro' import {Trans, useLingui} from '@lingui/react/macro'
import {useLingui} from '@lingui/react'
import {Trans} from '@lingui/react/macro'
import {atoms as a, useTheme, web} from '#/alf' import {atoms as a, useTheme, web} from '#/alf'
import {Button, ButtonText} from '#/components/Button' import {Button, ButtonText} from '#/components/Button'
@@ -22,7 +20,7 @@ export const enabled = createIsEnabledCheck(() => {
export function InviteFriendsAnnouncement() { export function InviteFriendsAnnouncement() {
const t = useTheme() const t = useTheme()
const {_} = useLingui() const {t: l} = useLingui()
const nuxDialogs = useNuxDialogContext() const nuxDialogs = useNuxDialogContext()
const control = Dialog.useDialogControl() const control = Dialog.useDialogControl()
@@ -37,9 +35,9 @@ export function InviteFriendsAnnouncement() {
// the invite dialog. The invite dialog is mounted persistently by NuxDialogs // the invite dialog. The invite dialog is mounted persistently by NuxDialogs
// (not here) so it survives the dismissal - the native bottom sheet cannot // (not here) so it survives the dismissal - the native bottom sheet cannot
// hand off to a second sheet mounted in this same subtree. Defer the open to // hand off to a second sheet mounted in this same subtree. Defer the open to
// the next tick so the announcement's teardown completes first. // the next frame so the announcement's teardown completes first.
control.close(() => { control.close(() => {
setTimeout(() => nuxDialogs.openInviteFriends()) requestAnimationFrame(() => nuxDialogs.openInviteFriends())
}) })
}, [control, nuxDialogs]) }, [control, nuxDialogs])
@@ -52,7 +50,7 @@ export function InviteFriendsAnnouncement() {
<Dialog.Handle fill={t.palette.primary_400} /> <Dialog.Handle fill={t.palette.primary_400} />
<Dialog.ScrollableInner <Dialog.ScrollableInner
label={_(msg`Invite your friends`)} label={l`Invite your friends`}
style={[web({maxWidth: 440})]} style={[web({maxWidth: 440})]}
contentContainerStyle={[ contentContainerStyle={[
{paddingTop: 0, paddingLeft: 0, paddingRight: 0}, {paddingTop: 0, paddingLeft: 0, paddingRight: 0},
@@ -72,9 +70,7 @@ export function InviteFriendsAnnouncement() {
accessibilityIgnoresInvertColors accessibilityIgnoresInvertColors
source={require('../../../../assets/images/invite_friends_announcement_nux.webp')} source={require('../../../../assets/images/invite_friends_announcement_nux.webp')}
style={[a.w_full, {aspectRatio: 754 / 440}]} style={[a.w_full, {aspectRatio: 754 / 440}]}
alt={_( alt={l`An illustration of the Bluesky app with a paper airplane flying out of it, representing inviting friends`}
msg`An illustration of the Bluesky app with a paper airplane flying out of it, representing inviting friends`,
)}
/> />
<View <View
style={[ style={[
@@ -121,7 +117,7 @@ export function InviteFriendsAnnouncement() {
{!IS_WEB && ( {!IS_WEB && (
<View style={[a.w_full, a.gap_sm]}> <View style={[a.w_full, a.gap_sm]}>
<Button <Button
label={_(msg`Try it`)} label={l`Try it`}
size="large" size="large"
color="primary" color="primary"
onPress={onPressTryIt} onPress={onPressTryIt}
@@ -131,7 +127,7 @@ export function InviteFriendsAnnouncement() {
</ButtonText> </ButtonText>
</Button> </Button>
<Button <Button
label={_(msg`Cancel`)} label={l`Cancel`}
size="large" size="large"
color="secondary" color="secondary"
onPress={() => control.close()} onPress={() => control.close()}
@@ -13,7 +13,6 @@ export function InviteFriendsDialog({
control={control} control={control}
onClose={onClose} onClose={onClose}
nativeOptions={{preventExpansion: true}}> nativeOptions={{preventExpansion: true}}>
{/* <Dialog.Handle /> */}
<InviteFriendsDialogInner control={control} /> <InviteFriendsDialogInner control={control} />
</Dialog.Outer> </Dialog.Outer>
) )
@@ -2,8 +2,7 @@ import {Suspense, useRef, useState} from 'react'
import {Pressable, View} from 'react-native' import {Pressable, View} from 'react-native'
import type ViewShot from 'react-native-view-shot' import type ViewShot from 'react-native-view-shot'
import {setStringAsync} from 'expo-clipboard' import {setStringAsync} from 'expo-clipboard'
import {requestMediaLibraryPermissionsAsync} from 'expo-image-picker' import {createAssetAsync, requestPermissionsAsync} from 'expo-media-library'
import {createAssetAsync} from 'expo-media-library'
import {useLingui} from '@lingui/react/macro' import {useLingui} from '@lingui/react/macro'
import {useNavigation} from '@react-navigation/native' import {useNavigation} from '@react-navigation/native'
@@ -78,7 +77,9 @@ export function InviteFriendsDialogInner({
return return
} }
const permission = await requestMediaLibraryPermissionsAsync() // Write-only permission - saving the QR image does not require read access
// to the user's photo library.
const permission = await requestPermissionsAsync(true)
if (!permission.granted) { if (!permission.granted) {
Toast.show( Toast.show(
l`You must grant access to your photo library to save the QR code`, l`You must grant access to your photo library to save the QR code`,
@@ -17,39 +17,47 @@ export function FollowersPromoBanner({
const t = useTheme() const t = useTheme()
return ( return (
<View style={[a.px_lg, a.pt_md]}> <View style={[a.px_lg, a.pt_md]}>
<Pressable {/*
accessibilityRole="button" * The dismiss button is a sibling of (not nested inside) the banner
accessibilityLabel={l`Find and invite friends`} * Pressable - nesting Pressables causes touch-handling conflicts on
accessibilityHint={l`Opens the find and invite friends settings`} * Android. This relative wrapper matches the banner bounds so the dismiss
onPress={onPress} * button can be positioned absolutely against the card's top-right corner.
style={({pressed}) => [ */}
a.flex_row, <View style={[a.relative]}>
a.align_center, <Pressable
a.w_full, accessibilityRole="button"
{ accessibilityLabel={l`Find and invite friends`}
height: 80, accessibilityHint={l`Opens the find and invite friends settings`}
borderRadius: 16, onPress={onPress}
backgroundColor: t.palette.primary_50, style={({pressed}) => [
paddingHorizontal: 16, a.flex_row,
paddingVertical: 16, a.align_center,
opacity: pressed ? 0.85 : 1, a.w_full,
}, {
]}> height: 80,
<Image borderRadius: 16,
accessibilityIgnoresInvertColors backgroundColor: t.palette.primary_50,
source={require('../../../../assets/images/invite_friends_promo_banner.webp')} paddingHorizontal: 16,
style={{width: 126, height: 64, marginRight: 16}} paddingVertical: 16,
contentFit="contain" opacity: pressed ? 0.85 : 1,
/> },
<Text
style={[
a.flex_1,
a.text_md,
a.font_bold,
{color: t.palette.primary_700, lineHeight: 19.5},
]}> ]}>
{l`Import contacts or invite your friends`} <Image
</Text> accessibilityIgnoresInvertColors
source={require('../../../../assets/images/invite_friends_promo_banner.webp')}
style={{width: 126, height: 64, marginRight: 16}}
contentFit="contain"
/>
<Text
style={[
a.flex_1,
a.text_md,
a.font_bold,
{color: t.palette.primary_700, lineHeight: 19.5},
]}>
{l`Import contacts or invite your friends`}
</Text>
</Pressable>
<Pressable <Pressable
accessibilityRole="button" accessibilityRole="button"
accessibilityLabel={l`Dismiss`} accessibilityLabel={l`Dismiss`}
@@ -67,7 +75,7 @@ export function FollowersPromoBanner({
]}> ]}>
<TimesIcon width={12} height={12} fill={t.palette.contrast_500} /> <TimesIcon width={12} height={12} fill={t.palette.contrast_500} />
</Pressable> </Pressable>
</Pressable> </View>
</View> </View>
) )
} }
+4 -4
View File
@@ -22,9 +22,9 @@ describe('invite URLs', () => {
) )
}) })
it('lowercases the handle', () => { it('passes the handle through verbatim (handles are API-normalized, like makeProfileLink)', () => {
expect(getInviteShareUrl('Alice.Bsky.Social')).toBe( expect(getInviteShareUrl('Alice.Bsky.Social')).toBe(
'https://bsky.app/profile/alice.bsky.social', 'https://bsky.app/profile/Alice.Bsky.Social',
) )
}) })
@@ -45,9 +45,9 @@ describe('invite URLs', () => {
expect(getInviteDisplayUrl('alice')).toBe('bsky.app/profile/alice') expect(getInviteDisplayUrl('alice')).toBe('bsky.app/profile/alice')
}) })
it('lowercases the handle', () => { it('passes the handle through verbatim (handles are API-normalized, like makeProfileLink)', () => {
expect(getInviteDisplayUrl('Danielle.bsky.team')).toBe( expect(getInviteDisplayUrl('Danielle.bsky.team')).toBe(
'bsky.app/profile/danielle.bsky.team', 'bsky.app/profile/Danielle.bsky.team',
) )
}) })
+1 -1
View File
@@ -16,7 +16,7 @@ function stripLeadingAt(handle: string): string {
/** Canonical URL - used for QR payload, Share, and Copy. Empty handle -> empty string. */ /** Canonical URL - used for QR payload, Share, and Copy. Empty handle -> empty string. */
export function getInviteShareUrl(handle: string): string { export function getInviteShareUrl(handle: string): string {
const bare = stripLeadingAt(handle).toLowerCase() const bare = stripLeadingAt(handle)
if (!bare) return '' if (!bare) return ''
return `https://bsky.app/profile/${bare}` return `https://bsky.app/profile/${bare}`
} }