From 389d9e2d310d94293931d3c9a909dd95de641abf Mon Sep 17 00:00:00 2001 From: Fredrik Burmester Date: Tue, 11 Nov 2025 09:07:34 +0100 Subject: [PATCH] fix(android): modal open when we don't want to --- app/(auth)/(tabs)/(libraries)/_layout.tsx | 236 +++++++++++++--------- components/PlatformDropdown.tsx | 64 +++--- docs/nested-modals.md | 38 +++- 3 files changed, 210 insertions(+), 128 deletions(-) diff --git a/app/(auth)/(tabs)/(libraries)/_layout.tsx b/app/(auth)/(tabs)/(libraries)/_layout.tsx index ebf77b66..acc7f817 100644 --- a/app/(auth)/(tabs)/(libraries)/_layout.tsx +++ b/app/(auth)/(tabs)/(libraries)/_layout.tsx @@ -1,5 +1,6 @@ import { Ionicons } from "@expo/vector-icons"; import { Stack } from "expo-router"; +import { useCallback, useEffect, useMemo, useState } from "react"; import { useTranslation } from "react-i18next"; import { Platform, View } from "react-native"; import { PlatformDropdown } from "@/components/PlatformDropdown"; @@ -8,9 +9,144 @@ import { useSettings } from "@/utils/atoms/settings"; export default function IndexLayout() { const { settings, updateSettings, pluginSettings } = useSettings(); + const [dropdownOpen, setDropdownOpen] = useState(false); const { t } = useTranslation(); + // Reset dropdown state when component unmounts or navigates away + useEffect(() => { + return () => { + setDropdownOpen(false); + }; + }, []); + + // Memoize callbacks to prevent recreating on every render + const handleDisplayRow = useCallback(() => { + updateSettings({ + libraryOptions: { + ...settings.libraryOptions, + display: "row", + }, + }); + }, [settings.libraryOptions, updateSettings]); + + const handleDisplayList = useCallback(() => { + updateSettings({ + libraryOptions: { + ...settings.libraryOptions, + display: "list", + }, + }); + }, [settings.libraryOptions, updateSettings]); + + const handleImageStylePoster = useCallback(() => { + updateSettings({ + libraryOptions: { + ...settings.libraryOptions, + imageStyle: "poster", + }, + }); + }, [settings.libraryOptions, updateSettings]); + + const handleImageStyleCover = useCallback(() => { + updateSettings({ + libraryOptions: { + ...settings.libraryOptions, + imageStyle: "cover", + }, + }); + }, [settings.libraryOptions, updateSettings]); + + const handleToggleTitles = useCallback(() => { + updateSettings({ + libraryOptions: { + ...settings.libraryOptions, + showTitles: !settings.libraryOptions.showTitles, + }, + }); + }, [settings.libraryOptions, updateSettings]); + + const handleToggleStats = useCallback(() => { + updateSettings({ + libraryOptions: { + ...settings.libraryOptions, + showStats: !settings.libraryOptions.showStats, + }, + }); + }, [settings.libraryOptions, updateSettings]); + + // Memoize groups to prevent recreating the array on every render + const dropdownGroups = useMemo( + () => [ + { + title: t("library.options.display"), + options: [ + { + type: "radio" as const, + label: t("library.options.row"), + value: "row", + selected: settings.libraryOptions.display === "row", + onPress: handleDisplayRow, + }, + { + type: "radio" as const, + label: t("library.options.list"), + value: "list", + selected: settings.libraryOptions.display === "list", + onPress: handleDisplayList, + }, + ], + }, + { + title: t("library.options.image_style"), + options: [ + { + type: "radio" as const, + label: t("library.options.poster"), + value: "poster", + selected: settings.libraryOptions.imageStyle === "poster", + onPress: handleImageStylePoster, + }, + { + type: "radio" as const, + label: t("library.options.cover"), + value: "cover", + selected: settings.libraryOptions.imageStyle === "cover", + onPress: handleImageStyleCover, + }, + ], + }, + { + title: "Options", + options: [ + { + type: "toggle" as const, + label: t("library.options.show_titles"), + value: settings.libraryOptions.showTitles, + onToggle: handleToggleTitles, + disabled: settings.libraryOptions.imageStyle === "poster", + }, + { + type: "toggle" as const, + label: t("library.options.show_stats"), + value: settings.libraryOptions.showStats, + onToggle: handleToggleStats, + }, + ], + }, + ], + [ + t, + settings.libraryOptions, + handleDisplayRow, + handleDisplayList, + handleImageStylePoster, + handleImageStyleCover, + handleToggleTitles, + handleToggleStats, + ], + ); + if (!settings?.libraryOptions) return null; return ( @@ -27,6 +163,8 @@ export default function IndexLayout() { !pluginSettings?.libraryOptions?.locked && !Platform.isTV && ( } title={t("library.options.display")} - groups={[ - { - title: t("library.options.display"), - options: [ - { - type: "radio", - label: t("library.options.row"), - value: "row", - selected: settings.libraryOptions.display === "row", - onPress: () => - updateSettings({ - libraryOptions: { - ...settings.libraryOptions, - display: "row", - }, - }), - }, - { - type: "radio", - label: t("library.options.list"), - value: "list", - selected: settings.libraryOptions.display === "list", - onPress: () => - updateSettings({ - libraryOptions: { - ...settings.libraryOptions, - display: "list", - }, - }), - }, - ], - }, - { - title: t("library.options.image_style"), - options: [ - { - type: "radio", - label: t("library.options.poster"), - value: "poster", - selected: - settings.libraryOptions.imageStyle === "poster", - onPress: () => - updateSettings({ - libraryOptions: { - ...settings.libraryOptions, - imageStyle: "poster", - }, - }), - }, - { - type: "radio", - label: t("library.options.cover"), - value: "cover", - selected: - settings.libraryOptions.imageStyle === "cover", - onPress: () => - updateSettings({ - libraryOptions: { - ...settings.libraryOptions, - imageStyle: "cover", - }, - }), - }, - ], - }, - { - title: "Options", - options: [ - { - type: "toggle", - label: t("library.options.show_titles"), - value: settings.libraryOptions.showTitles, - onToggle: () => - updateSettings({ - libraryOptions: { - ...settings.libraryOptions, - showTitles: !settings.libraryOptions.showTitles, - }, - }), - disabled: - settings.libraryOptions.imageStyle === "poster", - }, - { - type: "toggle", - label: t("library.options.show_stats"), - value: settings.libraryOptions.showStats, - onToggle: () => - updateSettings({ - libraryOptions: { - ...settings.libraryOptions, - showStats: !settings.libraryOptions.showStats, - }, - }), - }, - ], - }, - ]} + groups={dropdownGroups} /> ), }} diff --git a/components/PlatformDropdown.tsx b/components/PlatformDropdown.tsx index 62cbf23c..e0fa4378 100644 --- a/components/PlatformDropdown.tsx +++ b/components/PlatformDropdown.tsx @@ -191,35 +191,47 @@ const PlatformDropdownComponent = ({ const open = controlledOpen ?? internalOpen; const onOpenChange = controlledOnOpenChange ?? setInternalOpen; + // Track if modal is currently showing to prevent duplicate calls + const [isModalShowing, setIsModalShowing] = useState(false); + // Handle open/close state changes for Android useEffect(() => { - if (Platform.OS === "android" && open === true) { - showModal( - { - hideModal(); - onOpenChange?.(false); - }} - />, - { - snapPoints: ["90%"], - enablePanDownToClose: bottomSheetConfig?.enablePanDownToClose ?? true, - }, - ); + if (Platform.OS === "android") { + if (open === true && !isModalShowing) { + setIsModalShowing(true); + showModal( + { + setIsModalShowing(false); + hideModal(); + onOpenChange?.(false); + }} + />, + { + snapPoints: ["90%"], + enablePanDownToClose: + bottomSheetConfig?.enablePanDownToClose ?? true, + }, + ); + } else if (open === false && isModalShowing) { + setIsModalShowing(false); + hideModal(); + } } - }, [ - open, - title, - groups, - onOptionSelect, - onOpenChange, - bottomSheetConfig, - showModal, - hideModal, - ]); + }, [open]); + + // Cleanup on unmount + useEffect(() => { + return () => { + if (Platform.OS === "android" && isModalShowing) { + setIsModalShowing(false); + hideModal(); + } + }; + }, []); if (Platform.OS === "ios") { return ( diff --git a/docs/nested-modals.md b/docs/nested-modals.md index c0010fda..8cfdaef0 100644 --- a/docs/nested-modals.md +++ b/docs/nested-modals.md @@ -1,7 +1,7 @@ # Nested Modals with PlatformDropdown ## Issue -PlatformDropdowns inside BottomSheetModals don't open on Android. +PlatformDropdowns inside BottomSheetModals don't open on Android, or dropdowns reopen unexpectedly after navigation. ## Solution 1. **Add controlled state** for each PlatformDropdown: @@ -15,7 +15,34 @@ PlatformDropdowns inside BottomSheetModals don't open on Android. /> ``` -2. **Use `View` for triggers, not `TouchableOpacity`**: +2. **Memoize groups and callbacks** to prevent unnecessary re-renders: + ```tsx + const handleOption1 = useCallback(() => { + // handler logic + }, [dependencies]); + + const dropdownGroups = useMemo(() => [ + { + title: "Group 1", + options: [ + { + type: "radio", + label: "Option 1", + value: "option1", + selected: state === "option1", + onPress: handleOption1, + }, + ], + }, + ], [state, handleOption1]); + + + ``` + +3. **Use `View` for triggers, not `TouchableOpacity`**: ```tsx // ✅ Correct ``` -3. **Add `stackBehavior='push'` to parent BottomSheetModal**: +4. **Add `stackBehavior='push'` to parent BottomSheetModal**: ```tsx ``` -4. **Reset dropdown states on modal dismiss**: +5. **Reset dropdown states on modal dismiss**: ```tsx const handleDismiss = useCallback(() => { setDropdown1Open(false); @@ -53,6 +80,7 @@ PlatformDropdowns inside BottomSheetModals don't open on Android. ## Why - PlatformDropdown wraps triggers in TouchableOpacity on Android. Nested TouchableOpacity causes touch event conflicts. -- PlatformDropdown's useEffect should only call `showModal()` when `open === true`, not call `hideModal()` when `open === false` (interferes with parent modals). +- PlatformDropdown tracks internal `isModalShowing` state to prevent duplicate `showModal()` calls when dependencies change. +- Memoizing groups prevents the useEffect from re-triggering unnecessarily, which can cause modals to reopen after navigation. - Dropdown states must be reset on modal dismiss to prevent them from reopening automatically when parent modal reopens.