-
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
feat: New Profile Screen (read-only) #1519
Changes from all commits
257801e
b9e5b66
421fd3d
eb64fba
cf95354
679f608
2f847ab
2a3961f
8c499e2
11ae686
c998ad3
1cc0bf1
e1b68ae
d301be7
0d5ec12
32c4ba5
3b64b5c
ee8ed92
fe484ac
7e6dbbf
a889c73
6ae6e71
8c5a55b
0fdafe1
708b676
f9bdac9
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,80 @@ | ||
import React, { memo } from "react"; | ||
import { View, ViewStyle, Switch, TouchableOpacity } from "react-native"; | ||
import { Text } from "@/design-system/Text"; | ||
import { useAppTheme, ThemedStyle } from "@/theme/useAppTheme"; | ||
import { HStack } from "@/design-system/HStack"; | ||
import { VStack } from "@/design-system/VStack"; | ||
import { Icon } from "@/design-system/Icon/Icon"; | ||
import { ISettingsListRow } from "./settings-list.types"; | ||
|
||
type ISettingsListRowProps = { | ||
row: ISettingsListRow; | ||
editMode?: boolean; | ||
}; | ||
|
||
export const SettingsListRow = memo(function SettingsListRow({ | ||
row, | ||
editMode, | ||
}: ISettingsListRowProps) { | ||
const { theme, themed } = useAppTheme(); | ||
|
||
const content = ( | ||
<HStack style={themed($innerRowContainer)}> | ||
<VStack style={{ flex: 1 }}> | ||
<Text color={row.isWarning ? "caution" : "primary"}>{row.label}</Text> | ||
{row.value && ( | ||
<Text | ||
preset="formLabel" | ||
color={row.isWarning ? "caution" : "secondary"} | ||
> | ||
{row.value} | ||
</Text> | ||
)} | ||
</VStack> | ||
<View style={themed($rightContentContainer)}> | ||
{row.isSwitch ? ( | ||
<Switch | ||
value={!!row.value} | ||
onValueChange={row.onValueChange} | ||
disabled={row.disabled} | ||
/> | ||
) : ( | ||
<Icon | ||
icon="chevron.right" | ||
size={theme.iconSize.sm} | ||
color={theme.colors.text.secondary} | ||
/> | ||
)} | ||
</View> | ||
</HStack> | ||
); | ||
|
||
if (row.onPress) { | ||
return ( | ||
<TouchableOpacity style={themed($rowContainer)} onPress={row.onPress}> | ||
{content} | ||
</TouchableOpacity> | ||
); | ||
} | ||
|
||
return <HStack style={themed($rowContainer)}>{content}</HStack>; | ||
}); | ||
|
||
const $rowContainer: ThemedStyle<ViewStyle> = ({ spacing }) => ({ | ||
paddingVertical: spacing.md, | ||
width: "100%", | ||
flexDirection: "row", | ||
alignItems: "center", | ||
}); | ||
|
||
const $innerRowContainer: ThemedStyle<ViewStyle> = () => ({ | ||
width: "100%", | ||
justifyContent: "space-between", | ||
alignItems: "center", | ||
}); | ||
|
||
const $rightContentContainer: ThemedStyle<ViewStyle> = () => ({ | ||
marginLeft: "auto", | ||
alignItems: "flex-end", | ||
justifyContent: "center", | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
import React, { memo } from "react"; | ||
import { View, ViewStyle } from "react-native"; | ||
import { useAppTheme, ThemedStyle } from "@/theme/useAppTheme"; | ||
import { SettingsListRow } from "./settings-list-row"; | ||
import { ISettingsListRow } from "./settings-list.types"; | ||
|
||
type ISettingsListProps = { | ||
rows: ISettingsListRow[]; | ||
editMode?: boolean; | ||
}; | ||
|
||
export const SettingsList = memo(function SettingsList({ | ||
rows, | ||
editMode, | ||
}: ISettingsListProps) { | ||
const { themed } = useAppTheme(); | ||
|
||
return ( | ||
<View style={themed($container)}> | ||
{rows.map((row, index) => ( | ||
<SettingsListRow | ||
key={row.label + index} | ||
row={row} | ||
editMode={editMode} | ||
/> | ||
))} | ||
Comment on lines
+20
to
+26
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 Avoid using array index in key prop. Using array index as part of the key prop can lead to issues with component reordering and state preservation. Consider using a unique identifier from the row data instead. {rows.map((row, index) => (
<SettingsListRow
- key={row.label + index}
+ key={row.id || row.label} // Add an id property to ISettingsListRow
row={row}
editMode={editMode}
/>
))}
|
||
</View> | ||
); | ||
}); | ||
|
||
const $container: ThemedStyle<ViewStyle> = () => ({ | ||
width: "100%", | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
export type ISettingsListRow = { | ||
label: string; | ||
value?: string | boolean; | ||
onPress?: () => void; | ||
onValueChange?: (value: boolean) => void; | ||
isWarning?: boolean; | ||
isSwitch?: boolean; | ||
disabled?: boolean; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
import { Platform, Linking } from "react-native"; | ||
import { useAppStore } from "@/data/store/appStore"; | ||
import { requestPushNotificationsPermissions } from "../utils/requestPushNotificationsPermissions"; | ||
import { useSettingsStore } from "@/data/store/accountsStore"; | ||
|
||
type UseNotificationsPermissionReturn = { | ||
notificationsPermissionStatus: "granted" | "undetermined" | "denied"; | ||
requestPermission: () => Promise<void>; | ||
setNotificationsSettings: (settings: { | ||
showNotificationScreen: boolean; | ||
}) => void; | ||
}; | ||
|
||
export function useNotificationsPermission(): UseNotificationsPermissionReturn { | ||
const notificationsPermissionStatus = useAppStore( | ||
(s) => s.notificationsPermissionStatus | ||
); | ||
const setNotificationsPermissionStatus = useAppStore( | ||
(s) => s.setNotificationsPermissionStatus | ||
); | ||
const setNotificationsSettings = useSettingsStore( | ||
(s) => s.setNotificationsSettings | ||
); | ||
|
||
const requestPermission = async () => { | ||
if (notificationsPermissionStatus === "denied") { | ||
if (Platform.OS === "android") { | ||
// Android 13 is always denied first so let's try to show | ||
const newStatus = await requestPushNotificationsPermissions(); | ||
if (newStatus === "denied") { | ||
Linking.openSettings(); | ||
} else if (newStatus) { | ||
setNotificationsPermissionStatus(newStatus); | ||
} | ||
} else { | ||
Linking.openSettings(); | ||
} | ||
} else if (notificationsPermissionStatus === "undetermined") { | ||
// Open popup | ||
const newStatus = await requestPushNotificationsPermissions(); | ||
if (!newStatus) return; | ||
setNotificationsPermissionStatus(newStatus); | ||
} | ||
}; | ||
|
||
return { | ||
notificationsPermissionStatus, | ||
requestPermission, | ||
setNotificationsSettings, | ||
}; | ||
} | ||
Comment on lines
+1
to
+51
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. 💡 Codebase verification Migration needed for notification permission handling The new hook is properly used in UI components (Profile, NotificationsScreen, OnboardingNotificationsScreen) but several files still use direct permission management:
These files should be refactored to use the new hook for consistent permission handling across the codebase. 🔗 Analysis chainVerify the impact of this new hook across the codebase. Let's ensure all notification permission-related code is migrated to use this new hook. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for direct usage of notification permission-related functions
rg "requestPushNotificationsPermissions|setNotificationsPermissionStatus|setNotificationsSettings" --type ts
Length of output: 3684 Script: #!/bin/bash
# Check Profile.tsx implementation
rg "useNotificationsPermission|setNotificationsSettings" -B 2 -A 2 screens/Profile.tsx
Length of output: 586 |
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.
Not 100% sure about this variant and style 🤔
Wonder if we should just have around a for now?
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.
It's a button with no extra padding/margin, which helps align things. I used i in the ContactCard, but design has changed. Although this will be needed in the future!