-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Android Menu selector #1527
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
import { useCallback, useMemo } from "react"; | ||
import { | ||
MenuView, | ||
NativeActionEvent, | ||
MenuAction as RNMenuAction, | ||
} from "@react-native-menu/menu"; | ||
import { Platform, StyleProp, ViewStyle } from "react-native"; | ||
import { Pressable } from "../Pressable"; | ||
import { useAppTheme } from "@/theme/useAppTheme"; | ||
import { Haptics } from "@/utils/haptics"; | ||
import { getInlinedItem } from "./Menu.utils"; | ||
|
||
export type IMenuAction = Omit<RNMenuAction, "titleColor" | "imageColor"> & { | ||
color?: string; | ||
}; | ||
|
||
type MenuProps = { | ||
actions: IMenuAction[]; | ||
children: React.ReactNode; | ||
style?: StyleProp<ViewStyle>; | ||
onPress?: (actionId: string) => void; | ||
}; | ||
|
||
export const Menu = ({ children, actions, style, onPress }: MenuProps) => { | ||
const { theme } = useAppTheme(); | ||
|
||
const themedActions: RNMenuAction[] = useMemo(() => { | ||
return actions.map((action) => { | ||
const themedAction = { | ||
...action, | ||
// AR: Right now Android will only show a background color of white, so don't set a titleColor which is android specific anyways | ||
// This may be only an android 13 issue though, will either need to fix the library or wait for a fix | ||
titleColor: action.color ?? theme.colors.global.primary, | ||
imageColor: | ||
action.color ?? | ||
(Platform.OS === "android" ? theme.colors.global.primary : undefined), | ||
displayInline: action.displayInline, | ||
}; | ||
return action.displayInline ? getInlinedItem(themedAction) : themedAction; | ||
}); | ||
}, [actions, theme.colors.global.primary]); | ||
|
||
const handlePress = useCallback( | ||
({ nativeEvent }: NativeActionEvent) => { | ||
Haptics.selectionAsync(); | ||
onPress?.(nativeEvent.event); | ||
}, | ||
[onPress] | ||
); | ||
|
||
return ( | ||
<MenuView | ||
onPressAction={handlePress} | ||
actions={themedActions} | ||
style={style} | ||
shouldOpenOnLongPress={false} | ||
> | ||
<Pressable style={$pressable}>{children}</Pressable> | ||
</MenuView> | ||
); | ||
}; | ||
|
||
const $pressable: ViewStyle = { | ||
alignItems: "center", | ||
justifyContent: "center", | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import { Platform } from "react-native"; | ||
import { MenuAction as RNMenuAction } from "@react-native-menu/menu"; | ||
|
||
/** | ||
* Android will always make a sub menu item, so instead this will just return the item | ||
* iOS seems to work fine with the item, so this will just return the item | ||
* @param item RN Menu Action | ||
* @returns RN Menu Action | ||
*/ | ||
export const getInlinedItem = (item: RNMenuAction): RNMenuAction => { | ||
if (Platform.OS === "ios") { | ||
return { | ||
title: "", | ||
displayInline: true, | ||
subactions: [item], | ||
}; | ||
} | ||
return item; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,16 +14,12 @@ import { usePreferredName } from "@/hooks/usePreferredName"; | |
import { translate } from "@/i18n"; | ||
import { useHeader } from "@/navigation/use-header"; | ||
import { useAppTheme } from "@/theme/useAppTheme"; | ||
import { Haptics } from "@/utils/haptics"; | ||
import { shortDisplayName } from "@/utils/str"; | ||
import { useAccountsProfiles } from "@/utils/useAccountsProfiles"; | ||
import { useNavigation } from "@react-navigation/native"; | ||
import React from "react"; | ||
import { Alert, Platform } from "react-native"; | ||
import { | ||
ContextMenuButton, | ||
MenuActionConfig, | ||
} from "react-native-ios-context-menu"; | ||
import { Menu } from "@/design-system/Menu/Menu"; | ||
|
||
export function useHeaderWrapper() { | ||
const { theme } = useAppTheme(); | ||
|
@@ -84,80 +80,46 @@ export function useHeaderWrapper() { | |
<Avatar size={theme.avatarSize.sm} /> | ||
</Center> | ||
</Pressable> | ||
<ContextMenuButton | ||
// hitSlop={theme.spacing.sm} // Not working... | ||
<Menu | ||
style={{ | ||
paddingVertical: theme.spacing.sm, // TMP solution for the hitSlop not working | ||
paddingRight: theme.spacing.sm, // TMP solution for the hitSlop not working | ||
}} | ||
isMenuPrimaryAction | ||
onPressMenuItem={({ nativeEvent }) => { | ||
Haptics.selectionAsync(); | ||
if (nativeEvent.actionKey === "all-chats") { | ||
onPress={(actionId: string) => { | ||
Comment on lines
+83
to
+88
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Replace temporary styling solution. The comment indicates a temporary solution for hitSlop. Consider implementing a proper solution. - <Menu
- style={{
- paddingVertical: theme.spacing.sm, // TMP solution for the hitSlop not working
- paddingRight: theme.spacing.sm, // TMP solution for the hitSlop not working
- }}
+ <Menu
+ hitSlop={theme.spacing.sm}
+ pressableProps={{
+ style: {
+ padding: theme.spacing.xxs,
+ },
+ }}
|
||
if (actionId === "all-chats") { | ||
Alert.alert("Coming soon"); | ||
} else if (nativeEvent.actionKey === "new-account") { | ||
} else if (actionId === "new-account") { | ||
navigation.navigate("NewAccountNavigator"); | ||
} else if (nativeEvent.actionKey === "app-settings") { | ||
} else if (actionId === "app-settings") { | ||
Comment on lines
+89
to
+93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Implement proper feature flags for upcoming features. Using +const FEATURE_FLAGS = {
+ ALL_CHATS: false,
+ APP_SETTINGS: false,
+} as const;
+
+const handleUnimplementedFeature = (featureName: string) => {
+ Alert.alert(
+ translate("feature_coming_soon"),
+ translate("feature_coming_soon_description", { feature: featureName })
+ );
+};
onPress={(actionId: string) => {
if (actionId === "all-chats") {
- Alert.alert("Coming soon");
+ handleUnimplementedFeature("All Chats");
} else if (actionId === "new-account") {
navigation.navigate("NewAccountNavigator");
} else if (actionId === "app-settings") {
- Alert.alert("Coming soon");
+ handleUnimplementedFeature("App Settings");
}
|
||
Alert.alert("Coming soon"); | ||
} | ||
|
||
// Pressed on an account | ||
else { | ||
setCurrentAccount(nativeEvent.actionKey, false); | ||
setCurrentAccount(actionId, false); | ||
} | ||
}} | ||
menuConfig={{ | ||
menuTitle: "", | ||
menuItems: [ | ||
...accountsProfiles.map((profilePreferedName, index) => { | ||
return { | ||
actionKey: accounts[index], | ||
actionTitle: shortDisplayName(profilePreferedName), | ||
icon: { | ||
iconType: "SYSTEM", | ||
iconValue: | ||
currentAccount === accounts[index] | ||
? Platform.select({ | ||
default: "checkmark", | ||
ios: "checkmark", | ||
android: "checkmark", | ||
}) | ||
: "", | ||
}, | ||
} as MenuActionConfig; | ||
}), | ||
{ | ||
type: "menu", | ||
menuTitle: "", | ||
menuOptions: ["displayInline"], | ||
menuItems: [ | ||
{ | ||
actionKey: "new-account", | ||
actionTitle: translate("new_account"), | ||
icon: { | ||
iconType: "SYSTEM", | ||
iconValue: iconRegistry["new-account-card"], | ||
}, | ||
}, | ||
], | ||
}, | ||
{ | ||
type: "menu", | ||
menuTitle: "", | ||
menuOptions: ["displayInline"], | ||
menuItems: [ | ||
{ | ||
actionKey: "app-settings", | ||
actionTitle: translate("App settings"), | ||
icon: { | ||
iconType: "SYSTEM", | ||
iconValue: iconRegistry["settings"], | ||
}, | ||
}, | ||
], | ||
}, | ||
], | ||
}} | ||
actions={[ | ||
...accountsProfiles.map((profilePreferedName, index) => { | ||
return { | ||
id: accounts[index], | ||
title: shortDisplayName(profilePreferedName), | ||
image: currentAccount === accounts[index] ? "checkmark" : "", | ||
}; | ||
}), | ||
{ | ||
displayInline: true, | ||
id: "new-account", | ||
title: translate("new_account"), | ||
image: iconRegistry["new-account-card"], | ||
}, | ||
{ | ||
displayInline: true, | ||
id: "app-settings", | ||
title: translate("App settings"), | ||
image: iconRegistry["settings"], | ||
}, | ||
]} | ||
> | ||
<HStack | ||
// {...debugBorder()} | ||
|
@@ -181,7 +143,7 @@ export function useHeaderWrapper() { | |
/> | ||
</Center> | ||
</HStack> | ||
</ContextMenuButton> | ||
</Menu> | ||
</HStack> | ||
), | ||
}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we name this
DropdownMenu
instead please and the filedropdown-menu.tsx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry missed this, I will make an update for that