fix: artsy lens improvements and fixes

package.json

@@ -164,8 +164,8 @@
     "events": "3.3.0",
     "expo": "55.0.24",
     "expo-build-properties": "55.0.14",
-    "expo-file-system": "~55.0.20",
-    "expo-image-manipulator": "~55.0.20",
+    "expo-file-system": "55.0.20",
+    "expo-image-manipulator": "55.0.20",
     "expo-navigation-bar": "55.0.13",
     "expo-updates": "patch:expo-updates@npm%3A55.0.22#~/.yarn/patches/expo-updates-npm-55.0.22-990b55bddd.patch",
     "formik": "2.2.9",

src/app/Components/ArtworkGrids/ArtworkGridItem.tsx

@@ -56,14 +56,6 @@ export interface ArtworkProps extends ArtworkActionTrackingProps {
   artworkMetaStyle?: ViewProps["style"]
   disableArtworksListPrompt?: boolean
   disableProgressiveOnboarding?: boolean
-  /**
-   * Suppresses the RouterLink's own `to={artwork.href}` navigation, which otherwise fires on tap
-   * regardless of `onPress` -- `onPress` overrides only the default tracking/recent-searches side
-   * effect, not the navigation. For callers that navigate from `onPress` themselves because the
-   * href-based route can't resolve from where the grid is rendered, e.g. inside a modal presented
-   * outside the tab stack.
-   */
-  disableNavigation?: boolean
   /** Hide sale info */
   height?: number
   hideCuratorsPickSignal?: boolean
@@ -105,7 +97,6 @@ export const Artwork: React.FC<ArtworkProps> = memo(
     contextScreenOwnerType,
     contextScreenQuery,
     disableArtworksListPrompt = false,
-    disableNavigation = false,
     height,
     // disableProgressiveOnboarding = false,
     hideCuratorsPickSignal = false,
@@ -311,7 +302,6 @@ export const Artwork: React.FC<ArtworkProps> = memo(
             delayLongPress={400}
             navigationProps={navigationProps}
             to={artwork.href}
-            disableNavigation={disableNavigation}
             testID={`artworkGridItem-${artwork.title}`}
           >
             <View ref={itemRef}>

src/app/Components/ArtworkGrids/MasonryInfiniteScrollArtworkGrid.tsx

@@ -28,8 +28,6 @@ interface MasonryInfiniteScrollArtworkGridProps extends MasonryFlashListOmittedP
   /** Do not show add to artworks list prompt */
   disableArtworksListPrompt?: boolean
   disableProgressiveOnboarding?: boolean
-  /** See `ArtworkGridItem`'s `disableNavigation` doc comment. */
-  disableNavigation?: boolean
   hasMore?: boolean
   hideCreateAlertOnArtworkPreview?: boolean
   hideCuratorsPick?: boolean
@@ -69,7 +67,6 @@ export const MasonryInfiniteScrollArtworkGrid: React.FC<MasonryInfiniteScrollArt
   contextScreenOwnerType,
   disableArtworksListPrompt,
   disableProgressiveOnboarding,
-  disableNavigation,
   hasMore,
   hideCreateAlertOnArtworkPreview,
   hideCuratorsPick,
@@ -137,7 +134,6 @@ export const MasonryInfiniteScrollArtworkGrid: React.FC<MasonryInfiniteScrollArt
         hideSaveIcon={hideSaveIcon}
         disableArtworksListPrompt={disableArtworksListPrompt}
         disableProgressiveOnboarding={disableProgressiveOnboarding}
-        disableNavigation={disableNavigation}
         hideIncreasedInterestSignal={hideIncreasedInterest}
         hideCuratorsPickSignal={hideCuratorsPick}
         hideCreateAlertOnArtworkPreview={hideCreateAlertOnArtworkPreview}
@@ -167,7 +163,6 @@ export const MasonryInfiniteScrollArtworkGrid: React.FC<MasonryInfiniteScrollArt
       hideSaveIcon,
       disableArtworksListPrompt,
       disableProgressiveOnboarding,
-      disableNavigation,
       hideIncreasedInterest,
       hideCuratorsPick,
       hideCreateAlertOnArtworkPreview,

src/app/Components/GlobalSearchInput/__tests__/GlobalSearchInputOverlay.tests.tsx

@@ -31,22 +31,22 @@ const renderOverlay = (props: { hideModal?: () => void } = {}) =>
     { includeNavigation: true }
   )
 
-describe("GlobalSearchInputOverlay — Search by photo entry point", () => {
+describe("GlobalSearchInputOverlay — Search by Photo entry point", () => {
   const mockUseExperimentFlag = useExperimentFlag as jest.Mock
 
   beforeEach(() => {
     jest.clearAllMocks()
   })
 
-  it("renders the Search by photo button when onyx_artsy-lens is on", () => {
+  it("renders the Search by Photo button when onyx_artsy-lens is on", () => {
     mockUseExperimentFlag.mockImplementation((key) => key === "onyx_artsy-lens")
 
     renderOverlay()
 
     expect(screen.getByTestId("search-by-photo-button")).toBeOnTheScreen()
   })
 
-  it("hides the Search by photo button when onyx_artsy-lens is off", () => {
+  it("hides the Search by Photo button when onyx_artsy-lens is off", () => {
     mockUseExperimentFlag.mockReturnValue(false)
 
     renderOverlay()

src/app/Components/SearchByPhotoButton/SearchByPhotoButton.tsx

@@ -9,7 +9,7 @@ interface SearchByPhotoButtonProps {
   variant?: ButtonProps["variant"]
 }
 
-const LABEL = "Search by photo"
+const LABEL = "Search by Photo"
 
 const ICON_SIZE = 20
 

src/app/Components/SearchByPhotoButton/SearchByPhotoIconButton.tsx

@@ -14,7 +14,7 @@ export const SearchByPhotoIconButton: React.FC<SearchByPhotoIconButtonProps> = (
   return (
     <Touchable
       accessibilityRole="button"
-      accessibilityLabel="Search by photo"
+      accessibilityLabel="Search by Photo"
       onPress={onPress}
       hitSlop={ICON_HIT_SLOP}
       haptic="impactLight"

src/app/Components/SearchByPhotoButton/__tests__/SearchByPhotoButton.tests.tsx

@@ -3,7 +3,7 @@ import { SearchByPhotoButton } from "app/Components/SearchByPhotoButton/SearchBy
 import { renderWithWrappers } from "app/utils/tests/renderWithWrappers"
 import { StyleSheet } from "react-native"
 
-const labelColor = () => StyleSheet.flatten(screen.getByText("Search by photo").props.style).color
+const labelColor = () => StyleSheet.flatten(screen.getByText("Search by Photo").props.style).color
 
 describe("SearchByPhotoButton", () => {
   it("calls onPress", () => {

src/app/Navigation/__tests__/routes.tests.ts

@@ -1,3 +1,4 @@
+import { artsyDotNetRoutes } from "app/Navigation/routes"
 import { addRoute, addWebViewRoute } from "app/Navigation/utils/addRoute"
 import { matchRoute } from "app/system/navigation/utils/matchRoute"
 import { replaceParams } from "app/system/navigation/utils/replaceParams"
@@ -28,6 +29,18 @@ describe("artsy.net routes", () => {
     `)
   })
 
+  it("routes to Lens inside the active tab stack while hiding the bottom tabs", () => {
+    const lensRoute = artsyDotNetRoutes.find((route) => route.name === "Lens")
+
+    expect(matchRoute("/lens")).toEqual({
+      module: "Lens",
+      params: {},
+      type: "match",
+    })
+    expect(lensRoute?.options?.alwaysPresentModally).toBeUndefined()
+    expect(lensRoute?.options?.hidesBottomTabs).toBe(true)
+  })
+
   it("routes to Inbox", () => {
     expect(matchRoute("/inbox")).toMatchInlineSnapshot(`
       {

src/app/Navigation/routes.tsx

@@ -898,7 +898,6 @@ export const artsyDotNetRoutes = defineRoutes([
     name: "Lens",
     Component: Lens,
     options: {
-      alwaysPresentModally: true,
       hidesBottomTabs: true,
       screenOptions: { headerShown: false, gestureEnabled: false, animation: "fade" },
     },

src/app/Scenes/Lens/Components/LensCameraButtons.tsx

@@ -1,7 +1,6 @@
 import { BoltFillIcon, ImageSetIcon } from "@artsy/icons/native"
-import { Flex, FlexProps, useSpace } from "@artsy/palette-mobile"
+import { DEFAULT_HIT_SLOP, Flex, FlexProps, Touchable, useSpace } from "@artsy/palette-mobile"
 import { LensCapturePhotoButton } from "app/Scenes/Lens/Components/LensCapturePhotoButton"
-import { TouchableOpacity } from "react-native"
 
 export const LENS_CAMERA_BUTTONS_HEIGHT = 120
 
@@ -43,12 +42,13 @@ export const LensCameraButtons: React.FC<LensCameraButtonsProps> = (props) => {
       )}
 
       {mode === "camera" && !!deviceHasTorch && (
-        <TouchableOpacity
+        <Touchable
           testID="lens-torch-button"
           accessibilityRole="button"
           accessibilityLabel={isTorchEnabled ? "Turn off torch" : "Turn on torch"}
           onPress={onToggleTorch}
           disabled={!isCameraInitialized}
+          hitSlop={DEFAULT_HIT_SLOP}
           style={{ position: "absolute", right: space(2) }}
         >
           <Flex
@@ -63,14 +63,15 @@ export const LensCameraButtons: React.FC<LensCameraButtonsProps> = (props) => {
           >
             <BoltFillIcon fill={isTorchEnabled ? "mono100" : "mono0"} width={18} height={18} />
           </Flex>
-        </TouchableOpacity>
+        </Touchable>
       )}
 
-      <TouchableOpacity
+      <Touchable
         testID="lens-library-button"
         accessibilityRole="button"
         accessibilityLabel="Choose from library"
         onPress={onSelectFromLibrary}
+        hitSlop={DEFAULT_HIT_SLOP}
         style={
           mode === "camera" ? { position: "absolute", left: space(2) } : { position: "absolute" }
         }
@@ -85,7 +86,7 @@ export const LensCameraButtons: React.FC<LensCameraButtonsProps> = (props) => {
         >
           <ImageSetIcon fill="mono100" width={18} height={18} />
         </Flex>
-      </TouchableOpacity>
+      </Touchable>
     </Flex>
   )
 }

src/app/Scenes/Lens/Components/LensCameraErrorState.tsx

@@ -4,7 +4,7 @@ export const LensCameraErrorState: React.FC = () => {
   return (
     <Flex flex={1} bg="mono100" justifyContent="center" alignItems="center" px={4}>
       <Text variant="sm-display" color="mono0" textAlign="center">
-        Failed to open the camera device
+        Failed to open the camera
       </Text>
       <Text variant="xs" color="mono30" textAlign="center" mt={1}>
         You can still search using a photo from your library.

src/app/Scenes/Lens/Lens.tsx

@@ -14,10 +14,9 @@ const Stack = createStackNavigator<LensNavigationStack>()
  * modeled on MyCollectionArtworkForm.tsx's NavigationIndependentTree + NavigationContainer +
  * createStackNavigator setup.
  *
- * `detachInactiveScreens={false}` is load-bearing, not incidental: the deleted 2022
- * `ReverseImage.tsx` carries an all-caps warning that `react-native-screens` freezes the camera
- * and makes capture impossible inside a React Native modal (this screen is `alwaysPresentModally`
- * -> `fullScreenModal`, so that exact situation applies here). Do not remove it.
+ * `detachInactiveScreens={false}` is retained from the previous ReverseImage implementation, where
+ * detaching screens could freeze camera capture. Do not remove it without testing the full flow on
+ * physical iOS and Android devices.
  */
 export const Lens: React.FC = () => {
   const theme = useNavigationTheme()

src/app/Scenes/Lens/Screens/LensAnalyzing.tsx

@@ -9,7 +9,7 @@ import { LensNavigationStack } from "app/Scenes/Lens/types"
 import { CroppedPhoto, cropToViewfinder } from "app/Scenes/Lens/utils/cropToViewfinder"
 import { discardTempPhotos } from "app/Scenes/Lens/utils/discardTempPhotos"
 import { PhotoPresentation } from "app/Scenes/Lens/utils/viewfinderGeometry"
-import { dismissModal } from "app/system/navigation/navigate"
+import { goBack } from "app/system/navigation/navigate"
 import { uploadImageToS3 } from "app/utils/uploadImageToS3"
 import { useEffect, useRef, useState } from "react"
 import { Image, useWindowDimensions } from "react-native"
@@ -19,34 +19,19 @@ type Props = StackScreenProps<LensNavigationStack, "LensAnalyzing">
 const CARD_HORIZONTAL_MARGIN = 32
 const CARD_MAX_HEIGHT_FRACTION = 0.62
 
-/**
- * Crops the photo to the area the brackets marked, uploads the *cropped* file, then hands the
- * resulting `s3Bucket`/`s3Key` to `LensResults`.
- *
- * It displays the cropped file rather than the original with a bracket overlay: an overlay marking
- * an inset sub-region always leaves image content visible outside the brackets, however accurate
- * the crop math. So the crop runs up front and its URI is reused for the upload.
- *
- * The crop's reference container differs by capture method, and passing the wrong one reproduces
- * the crop-doesn't-match-brackets bug. A camera capture was framed against `LensCamera`'s measured
- * viewport (`photo.captureContainerWidth`/`Height`); a library pick was only ever shown inside this
- * screen's preview card, so that card's dimensions are its container.
- */
 export const LensAnalyzing: React.FC<Props> = ({ route, navigation }) => {
   const { photo } = route.params
   const { width: windowWidth, height: windowHeight } = useWindowDimensions()
   const [cropped, setCropped] = useState<CroppedPhoto | null>(null)
   const [hasError, setHasError] = useState(false)
 
-  // Temp files to sweep on the way out (see the effect below). A ref, not state: adding them to
-  // the crop effect's deps would re-run the crop and upload. A library pick's file is deliberately
-  // left out -- see `discardTempPhotos` on why deleting one can cost the user their actual photo.
+  // A ref avoids re-running the crop effect when a temporary file is added. Library photos must
+  // not be deleted because their URI can point to the user's original file.
   const tempPhotoUris = useRef<string[]>(photo.fromLibrary ? [] : [photo.uri])
 
   const maxCardWidth = windowWidth - CARD_HORIZONTAL_MARGIN * 2
   const maxCardHeight = windowHeight * CARD_MAX_HEIGHT_FRACTION
 
-  /** The largest box of the given aspect ratio that fits the space reserved for the card. */
   const fitCard = (aspectRatio: number) => {
     let width = maxCardWidth
     let height = width / aspectRatio
@@ -59,28 +44,21 @@ export const LensAnalyzing: React.FC<Props> = ({ route, navigation }) => {
     return { width, height }
   }
 
-  // Two separate sizes on purpose. `viewfinderCard` is what a library pick is cropped against, so
-  // it must NOT depend on the crop's dimensions -- that would make the crop container depend on
-  // the crop result and re-trigger the effect on every resolve. `displayCard` is presentational:
-  // once the crop lands the card takes its actual shape, so a sideways capture gets a landscape
-  // card instead of letterbox bars.
+  // The crop container must not depend on the crop result, or the effect will re-run after cropping.
   const viewfinderCard = fitCard(LENS_VIEWFINDER_ASPECT_RATIO)
   const displayCard =
     cropped && cropped.width > 0 && cropped.height > 0
       ? fitCard(cropped.width / cropped.height)
       : viewfinderCard
 
-  // The window fallback is defensive, for a photo that arrives without the measurement.
   const cropContainerWidth = photo.fromLibrary
     ? viewfinderCard.width
     : photo.captureContainerWidth ?? windowWidth
   const cropContainerHeight = photo.fromLibrary
     ? viewfinderCard.height
     : photo.captureContainerHeight ?? windowHeight
 
-  // A camera capture was framed against a preview that aligns the sensor feed to the
-  // orientation-locked UI, so the crop has to invert that rotation. `<Image resizeMode="cover">`
-  // never rotates, so a library pick must not be. See `PhotoPresentation`.
+  // Camera captures account for the preview rotation; library photos use regular cover geometry.
   const cropPresentation: PhotoPresentation = photo.fromLibrary
     ? "cover"
     : "coverRotatedToContainer"
@@ -90,8 +68,7 @@ export const LensAnalyzing: React.FC<Props> = ({ route, navigation }) => {
 
     cropToViewfinder(photo.uri, cropContainerWidth, cropContainerHeight, cropPresentation)
       .then((cropped) => {
-        // Before the cancellation check, not after: the file is on disk either way, and an
-        // abandoned attempt is precisely what nothing else would clean up.
+        // Track the file before checking cancellation so an abandoned crop is still cleaned up.
         tempPhotoUris.current.push(cropped.uri)
 
         if (cancelled) {
@@ -127,8 +104,7 @@ export const LensAnalyzing: React.FC<Props> = ({ route, navigation }) => {
 
   useEffect(() => {
     return () => {
-      // The lint rule wants this copied into a variable at setup time, which is the one thing
-      // that would break it: the crop is appended after setup, so copying leaks every crop.
+      // Copying the ref during setup would miss files appended after the crop completes.
       // eslint-disable-next-line react-hooks/exhaustive-deps
       discardTempPhotos(tempPhotoUris.current)
     }
@@ -138,7 +114,7 @@ export const LensAnalyzing: React.FC<Props> = ({ route, navigation }) => {
     <Theme theme="v3light">
       <Flex flex={1} bg="mono100" justifyContent="center" alignItems="center">
         <Flex position="absolute" top={0} left={0} right={0}>
-          <LensHeader onClose={() => dismissModal()} />
+          <LensHeader onClose={() => goBack()} />
         </Flex>
 
         {hasError ? (
@@ -151,7 +127,6 @@ export const LensAnalyzing: React.FC<Props> = ({ route, navigation }) => {
               <SearchByPhotoButton
                 testID="lensAnalyzingSearchByPhotoButton"
                 variant="fillLight"
-                // `navigate`, not `push`: LensCamera is this stack's root and still mounted below.
                 onPress={() => navigation.navigate("LensCamera")}
               />
             </Flex>

src/app/Scenes/Lens/Screens/LensCamera.tsx

@@ -16,11 +16,11 @@ import { LensCornerBrackets } from "app/Scenes/Lens/Components/LensCornerBracket
 import { LensHeader } from "app/Scenes/Lens/Components/LensHeader"
 import { LensPermissionPlaceholder } from "app/Scenes/Lens/Components/LensPermissionPlaceholder"
 import { LensNavigationStack } from "app/Scenes/Lens/types"
-import { dismissModal } from "app/system/navigation/navigate"
+import { goBack } from "app/system/navigation/navigate"
 import { requestPhotos } from "app/utils/requestPhotos"
-import { useIsForeground } from "app/utils/useIsForeground"
+import useAppState from "app/utils/useAppState"
 import { useRef, useState } from "react"
-import { Linking } from "react-native"
+import { AppState, Linking } from "react-native"
 
 type Props = StackScreenProps<LensNavigationStack, "LensCamera">
 
@@ -39,8 +39,9 @@ export const LensCamera: React.FC<Props> = ({ navigation }) => {
   const [containerSize, setContainerSize] = useState({ width: 0, height: 0 })
 
   const isFocused = useIsFocused()
-  const isForeground = useIsForeground()
-  const isActive = isFocused && isForeground
+  const [appState, setAppState] = useState(AppState.currentState)
+  useAppState({ onChange: setAppState })
+  const isActive = isFocused && appState === "active"
 
   const reportError = (context: string, error: unknown) => {
     if (__DEV__) {
@@ -54,7 +55,7 @@ export const LensCamera: React.FC<Props> = ({ navigation }) => {
   }
 
   const handleClose = () => {
-    dismissModal()
+    goBack()
   }
 
   const handleTakePhoto = () => {