Skip to content
Open
8 changes: 6 additions & 2 deletions src/components/Lightbox/chrome/CircleChromeButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,13 @@ type Props = {
| 'accessibilityHint'
>

const SIZE = 32
/**
* Apple's minimum tap size. The image options button is a native menu trigger
* on iOS, where `hitSlop` doesn't apply, so the circle itself is the target.
*/
const SIZE = 44
const RADIUS = SIZE / 2
const ICON = 18
const ICON = 24

export function CircleChromeButton({
icon: Icon,
Expand Down
224 changes: 26 additions & 198 deletions src/components/Lightbox/chrome/ImageMenu.tsx
Original file line number Diff line number Diff line change
@@ -1,217 +1,45 @@
import {useEffect, useRef, useState} from 'react'
import {
Modal,
Pressable,
StyleSheet,
useWindowDimensions,
View,
} from 'react-native'
import Animated, {
interpolate,
useAnimatedStyle,
useSharedValue,
withSpring,
withTiming,
} from 'react-native-reanimated'
import {scheduleOnRN} from 'react-native-worklets'
import {useLingui} from '@lingui/react/macro'

import {atoms as a} from '#/alf'
import {ThemeProvider} from '#/alf'
import {ArrowShareRight_Stroke2_Corner2_Rounded as ShareIcon} from '#/components/icons/ArrowShareRight'
import {type Props as IconProps} from '#/components/icons/common'
import {DotGrid3x1_Stroke2_Corner0_Rounded as DotsIcon} from '#/components/icons/DotGrid'
import {Download_Stroke2_Corner0_Rounded as DownloadIcon} from '#/components/icons/Download'
import {Text} from '#/components/Typography'
import {IS_IPAD} from '#/env'
import * as Menu from '#/components/Menu'
import {CircleChromeButton} from './CircleChromeButton'

type Props = {
onPressShare: () => void
onPressSave: () => void
}

type Anchor = {x: number; y: number; width: number; height: number}

const GAP = 6
const CARD_BG = '#000000'
const CARD_BORDER = '#232e3e'
const ITEM_TEXT = '#f9fafb'

const SPRING_IN = {damping: 18, mass: 0.6, stiffness: 240}
const TIMING_OUT = {duration: 150}

export function ImageMenu({onPressShare, onPressSave}: Props) {
const {t: l} = useLingui()
const triggerRef = useRef<React.ComponentRef<typeof View>>(null)
const [isMounted, setIsMounted] = useState(false)
const [anchor, setAnchor] = useState<Anchor | null>(null)
const {width: windowWidth, height: windowHeight} = useWindowDimensions()
const progress = useSharedValue(0)

useEffect(() => {
if (!isMounted) return
triggerRef.current?.measureInWindow((x, y, width, height) => {
setAnchor({x, y, width, height})
})
}, [isMounted, windowWidth, windowHeight])

const open = () => {
triggerRef.current?.measureInWindow((x, y, width, height) => {
setAnchor({x, y, width, height})
setIsMounted(true)
progress.set(withSpring(1, SPRING_IN))
})
}

const close = () => {
progress.set(
withTiming(0, TIMING_OUT, finished => {
if (finished) {
scheduleOnRN(setIsMounted, false)
}
}),
)
}

const runAction = (action: () => void) => {
close()
action()
}

return (
<>
<View ref={triggerRef} collapsable={false}>
<CircleChromeButton
icon={DotsIcon}
iconStyle={{transform: [{rotate: '90deg'}]}}
label={l`Image options`}
onPress={open}
/>
</View>
<Modal
transparent
visible={isMounted}
animationType="none"
onRequestClose={close}
supportedOrientations={
IS_IPAD
? [
'portrait',
'portrait-upside-down',
'landscape-left',
'landscape-right',
]
: ['portrait', 'landscape']
}
statusBarTranslucent>
<Pressable
accessibilityRole="button"
accessibilityLabel={l`Close menu`}
accessibilityHint=""
style={[a.absolute, a.inset_0]}
onPress={close}
/>
{anchor && (
<MenuCard anchor={anchor} progress={progress}>
<MenuItem
icon={ShareIcon}
label={l`Share image`}
onPress={() => runAction(onPressShare)}
// The lightbox is always dark, so its menu is too.
<ThemeProvider theme="dark">
Comment on lines +19 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking: With the gate off, the image menu becomes a light sheet with dark-theme items

Menu.Root inline only renders a native menu when NativeMenusEnable is on (src/components/Menu/index.tsx:78-79). Otherwise it falls back to Dialog.Outer. That gate is off by default, so most users will see the fallback. The native Dialog.Outer takes the sheet's backgroundColor and colorScheme from useTheme(useThemeName()) (src/components/Dialog/index.tsx:90-91, 301-302), which is the app's colour mode. It ignores this ThemeProvider. The items inside use plain useTheme(), so they do get the dark theme.

How to reproduce: use the app in light mode with the gate off, open an image in the lightbox, and tap …. You get a white sheet, a light-mode handle, and dark bg_contrast_25 item cards with light text. The old hand-rolled card was dark in every colour mode.

Possible fixes: apply the ThemeProvider only when the menu really is inline, or keep the old dark card for the fallback until the gate rolls out.

<Menu.Root inline>
<Menu.Trigger label={l`Image options`}>
{({props}) => (
<CircleChromeButton
icon={DotsIcon}
iconStyle={{transform: [{rotate: '90deg'}]}}
label={props.accessibilityLabel}
onPress={props.onPress}
/>
<MenuItem
icon={DownloadIcon}
label={l`Save image`}
onPress={() => runAction(onPressSave)}
/>
</MenuCard>
)}
</Modal>
</>
)
}

function MenuCard({
anchor,
progress,
children,
}: {
anchor: Anchor
progress: ReturnType<typeof useSharedValue<number>>
children: React.ReactNode
}) {
const animatedStyle = useAnimatedStyle(() => ({
opacity: progress.get(),
transform: [{scale: interpolate(progress.get(), [0, 1], [0.9, 1])}],
}))

return (
<Animated.View
style={[
a.absolute,
a.self_start,
styles.card,
{
top: anchor.y + anchor.height + GAP,
left: anchor.x,
transformOrigin: 'top left',
},
animatedStyle,
]}>
{children}
</Animated.View>
)}
</Menu.Trigger>
<Menu.Outer>
<Menu.Item label={l`Share image`} onPress={onPressShare}>
<Menu.ItemText>{l`Share image`}</Menu.ItemText>
<Menu.ItemIcon icon={ShareIcon} />
</Menu.Item>
<Menu.Item label={l`Save image`} onPress={onPressSave}>
<Menu.ItemText>{l`Save image`}</Menu.ItemText>
<Menu.ItemIcon icon={DownloadIcon} />
</Menu.Item>
</Menu.Outer>
</Menu.Root>
</ThemeProvider>
)
}

function MenuItem({
icon: Icon,
label,
onPress,
}: {
icon: React.ComponentType<IconProps>
label: string
onPress: () => void
}) {
return (
<Pressable
accessibilityRole="button"
accessibilityLabel={label}
accessibilityHint=""
onPress={onPress}
style={({pressed}) => [styles.item, pressed && styles.itemPressed]}>
<Icon width={18} fill={ITEM_TEXT} />
<Text style={styles.itemText}>{label}</Text>
</Pressable>
)
}

const styles = StyleSheet.create({
card: {
padding: 8,
borderRadius: 16,
borderWidth: 0.5,
borderColor: CARD_BORDER,
backgroundColor: CARD_BG,
shadowColor: '#000',
shadowOffset: {width: 0, height: 4},
shadowOpacity: 0.16,
shadowRadius: 20,
elevation: 8,
},
item: {
height: 44,
paddingHorizontal: 12,
borderRadius: 8,
flexDirection: 'row',
alignItems: 'center',
gap: 6,
},
itemPressed: {
backgroundColor: 'rgba(255, 255, 255, 0.08)',
},
itemText: {
fontSize: 15,
fontWeight: '500',
lineHeight: 19.5,
color: ITEM_TEXT,
},
})
6 changes: 1 addition & 5 deletions src/components/PostControls/RepostButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,7 @@ jest.mock('#/components/PostControls/RepostButtonMenu', () => {
const {View} = require('react-native')

return {
RepostButtonMenu: ({longPressToQuote}: {longPressToQuote?: boolean}) => (
<View
testID={longPressToQuote ? 'ipad-repost-menu' : 'web-repost-menu'}
/>
),
RepostButtonMenu: () => <View testID="ipad-repost-menu" />,
}
})
jest.mock('#/components/Dialog', () => {
Expand Down
6 changes: 1 addition & 5 deletions src/components/PostControls/RepostButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,7 @@ import {
import {RepostButtonMenu, type RepostButtonProps} from './RepostButtonMenu'

let RepostButton = (props: RepostButtonProps): React.ReactNode =>
IS_IPAD ? (
<RepostButtonMenu {...props} longPressToQuote />
) : (
<RepostButtonDialog {...props} />
)
IS_IPAD ? <RepostButtonMenu {...props} /> : <RepostButtonDialog {...props} />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking: Long press on the iPad repost button does nothing when the gate is off

Removing longPressToQuote assumes a long press opens the native menu. That only happens when NativeMenusEnable is on. With the gate off (the default), Menu.Root inline falls back to the bottom sheet (src/components/Menu/index.tsx:78-82). The trigger is now just PostControlButton {...props} with no onLongPress, so on iPad a long press neither quotes the post (the old behaviour) nor opens the menu. Today that's a silent regression for every iPad user.

Possible fix: keep the long-press handler for the non-inline case, for example by reading useMenuContext().inline in the trigger, until the gate is fully rolled out.


RepostButton = memo(RepostButton)
export {RepostButton}
Expand Down
52 changes: 2 additions & 50 deletions src/components/PostControls/RepostButtonMenu.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {fireEvent, render, within} from '@testing-library/react-native'

import * as Menu from '#/components/Menu'
import {RepostButtonMenu} from './RepostButtonMenu'

jest.mock('#/state/session', () => ({
Expand All @@ -12,12 +11,8 @@ jest.mock('#/alf', () => ({
}))
jest.mock('#/components/Menu', () => {
const {Pressable, Text, View} = require('react-native')
const open = jest.fn()
const unanchoredOpen = jest.fn()

return {
__testOpen: open,
__testUnanchoredOpen: unanchoredOpen,
Root: ({children}: React.PropsWithChildren) => <View>{children}</View>,
Trigger: ({
children,
Expand All @@ -30,8 +25,8 @@ jest.mock('#/components/Menu', () => {
label: string
}) =>
children({
control: {open: unanchoredOpen},
props: {accessibilityLabel: label, onPress: open},
control: {open: jest.fn()},
props: {accessibilityLabel: label, onPress: jest.fn()},
}),
Outer: ({children}: React.PropsWithChildren) => <View>{children}</View>,
Group: ({children}: React.PropsWithChildren) => (
Expand Down Expand Up @@ -73,19 +68,16 @@ jest.mock('./PostControlButton', () => {
PostControlButton: ({
children,
label,
onLongPress,
onPress,
testID,
}: React.PropsWithChildren<{
label?: string
onLongPress?: () => void
onPress?: () => void
testID?: string
}>) => (
<Pressable
accessibilityHint=""
accessibilityLabel={label}
onLongPress={onLongPress}
onPress={onPress}
testID={testID}>
{children}
Expand All @@ -110,11 +102,6 @@ jest.mock('@lingui/react', () => ({
}),
}))

const mockMenuOpen = (Menu as unknown as {__testOpen: jest.Mock}).__testOpen
const mockUnanchoredOpen = (
Menu as unknown as {__testUnanchoredOpen: jest.Mock}
).__testUnanchoredOpen

afterEach(() => {
jest.clearAllMocks()
})
Expand Down Expand Up @@ -158,39 +145,4 @@ describe('RepostButtonMenu', () => {
fireEvent.press(quoteButton)
expect(onQuote).not.toHaveBeenCalled()
})

it('opens the menu on long press when quote is disabled', () => {
const screen = render(
<RepostButtonMenu
isReposted={false}
onRepost={jest.fn()}
onQuote={jest.fn()}
embeddingDisabled
longPressToQuote
/>,
)

fireEvent(screen.getByTestId('repostBtn'), 'longPress')

expect(mockMenuOpen).toHaveBeenCalledTimes(1)
expect(mockUnanchoredOpen).not.toHaveBeenCalled()
})

it('keeps long press-to-quote when quote is enabled', () => {
const onQuote = jest.fn()
const screen = render(
<RepostButtonMenu
isReposted={false}
onRepost={jest.fn()}
onQuote={onQuote}
embeddingDisabled={false}
longPressToQuote
/>,
)

fireEvent(screen.getByTestId('repostBtn'), 'longPress')

expect(onQuote).toHaveBeenCalledTimes(1)
expect(mockMenuOpen).not.toHaveBeenCalled()
})
})
Loading
Loading