-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Add new Profile page #20144
Add new Profile page #20144
Changes from all commits
e1ab57e
558b23c
540087d
09e6b1f
e138aee
249369e
2d9e221
7f5f165
f5832a3
b1f7118
05d5979
282d9e3
516daeb
b28bf1c
a9631fc
c1d5958
61976f1
8b4f566
afb818c
922a505
c27e85c
d77d9b5
74b5ed2
133cd8e
3ddc601
57beb1d
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 |
---|---|---|
|
@@ -55,6 +55,7 @@ const ReportWelcomeText = (props) => { | |
const isChatRoom = ReportUtils.isChatRoom(props.report); | ||
const isDefault = !(isChatRoom || isPolicyExpenseChat); | ||
const participants = lodashGet(props.report, 'participants', []); | ||
const participantAccountIDs = lodashGet(props.report, 'participantAccountIDs', []); | ||
const isMultipleParticipant = participants.length > 1; | ||
const displayNamesWithTooltips = ReportUtils.getDisplayNamesWithTooltips(OptionsListUtils.getPersonalDetailsForLogins(participants, props.personalDetails), isMultipleParticipant); | ||
const roomWelcomeMessage = ReportUtils.getRoomWelcomeMessage(props.report); | ||
|
@@ -97,7 +98,7 @@ const ReportWelcomeText = (props) => { | |
<Tooltip text={tooltip}> | ||
<Text | ||
style={[styles.textStrong]} | ||
onPress={() => Navigation.navigate(ROUTES.getDetailsRoute(participants[index]))} | ||
onPress={() => Navigation.navigate(ROUTES.getProfileRoute(participantAccountIDs[index]))} | ||
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. This was overlooked. 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. #20583 (comment) for more details about the root cause. 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. Oh wow, that makes total sense. Should've considered that. Thanks for the feedback! |
||
> | ||
{displayName} | ||
</Text> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -310,6 +310,49 @@ function openPersonalDetailsPage() { | |
API.read('OpenPersonalDetailsPage'); | ||
} | ||
|
||
/** | ||
* Fetches public profile info about a given user. | ||
* The API will only return the accountID, displayName, and avatar for the user | ||
* but the profile page will use other info (e.g. contact methods and pronouns) if they are already available in Onyx | ||
* @param {Number} accountID | ||
*/ | ||
function openPublicProfilePage(accountID) { | ||
const optimisticData = [ | ||
{ | ||
onyxMethod: Onyx.METHOD.MERGE, | ||
key: ONYXKEYS.PERSONAL_DETAILS_LIST, | ||
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. Adding |
||
value: { | ||
[accountID]: { | ||
isLoading: true, | ||
}, | ||
}, | ||
}, | ||
]; | ||
const successData = [ | ||
{ | ||
onyxMethod: Onyx.METHOD.MERGE, | ||
key: ONYXKEYS.PERSONAL_DETAILS_LIST, | ||
value: { | ||
[accountID]: { | ||
isLoading: false, | ||
}, | ||
}, | ||
}, | ||
]; | ||
const failureData = [ | ||
{ | ||
onyxMethod: Onyx.METHOD.MERGE, | ||
key: ONYXKEYS.PERSONAL_DETAILS_LIST, | ||
value: { | ||
[accountID]: { | ||
isLoading: false, | ||
}, | ||
}, | ||
}, | ||
]; | ||
API.read('OpenPublicProfilePage', {accountID}, {optimisticData, successData, failureData}); | ||
} | ||
|
||
/** | ||
* Updates the user's avatar image | ||
* | ||
|
@@ -427,6 +470,7 @@ export { | |
deleteAvatar, | ||
openMoneyRequestModalPage, | ||
openPersonalDetailsPage, | ||
openPublicProfilePage, | ||
extractFirstAndLastNameFromAvailableDetails, | ||
updateDisplayName, | ||
updateLegalName, | ||
|
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.
👋 Just a heads-up that this has caused a regression in #21969
Deep linking wasn't working for the new PROFILE route.
When adding a new route we should also add it to App/well-known
/apple-app-site-association for iOS
And to AndroidManifest.xml for Android
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 wow. I had no idea. Can we make that obvious somehow? Like with a checklist item or lint rule or something.
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.
@puneetlath, I proposed to add a checklist item in https://expensify.slack.com/archives/C049HHMV9SM/p1690395182200269
Looks like we should add it, I'll do that tomorrow (not sure what the process is just yet, will find it)