Skip to content
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

Merged
merged 26 commits into from
Jun 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
e1ab57e
Add route and links for new profile page
puneetlath Jun 3, 2023
558b23c
Add ProfilePage
puneetlath Jun 3, 2023
540087d
Use lodashGet for various details
puneetlath Jun 3, 2023
09e6b1f
Show loading indicator when details arent available
puneetlath Jun 3, 2023
e138aee
Use "hidden" as fallback displayName
puneetlath Jun 3, 2023
249369e
Replace getDetailsRoute usage with getProfileRoute
puneetlath Jun 4, 2023
2d9e221
Fetch the personal details from the API when opening profile page
puneetlath Jun 4, 2023
7f5f165
Dont show timezone if we dont have it
puneetlath Jun 4, 2023
f5832a3
Dont show message option if you dont have their login info
puneetlath Jun 4, 2023
b1f7118
Use avatar that we have
puneetlath Jun 4, 2023
05d5979
Use accountID for default avatar if no login
puneetlath Jun 5, 2023
282d9e3
Merge main into branch
puneetlath Jun 5, 2023
516daeb
Use new profile for report participants
puneetlath Jun 5, 2023
b28bf1c
Make component functional
puneetlath Jun 5, 2023
a9631fc
Fix lint error
puneetlath Jun 5, 2023
c1d5958
Add failed to load view
puneetlath Jun 5, 2023
61976f1
Dont make subtitle requied on BlockingView
puneetlath Jun 6, 2023
8b4f566
Fix not found page showing after loading
puneetlath Jun 6, 2023
afb818c
Merge main into branch
puneetlath Jun 7, 2023
922a505
Re-add personalDetailsList onyx key
puneetlath Jun 7, 2023
c27e85c
Update src/libs/actions/PersonalDetails.js
puneetlath Jun 7, 2023
d77d9b5
Update src/libs/actions/PersonalDetails.js
puneetlath Jun 7, 2023
74b5ed2
Update src/libs/actions/PersonalDetails.js
puneetlath Jun 7, 2023
133cd8e
Alex comments
puneetlath Jun 8, 2023
3ddc601
Update src/pages/ProfilePage.js
puneetlath Jun 8, 2023
57beb1d
Prettier fix
puneetlath Jun 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/ONYXKEYS.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ export default {
// Contains all the personalDetails the user has access to
PERSONAL_DETAILS: 'personalDetails',

// Contains all the personalDetails the user has access to, keyed by accountID
PERSONAL_DETAILS_LIST: 'personalDetailsList',

// Contains all the private personal details of the user
PRIVATE_PERSONAL_DETAILS: 'private_personalDetails',

Expand Down
6 changes: 4 additions & 2 deletions src/ROUTES.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,12 @@ export default {
SET_PASSWORD_WITH_VALIDATE_CODE: 'setpassword/:accountID/:validateCode',
DETAILS: 'details',
getDetailsRoute: (login) => `details?login=${encodeURIComponent(login)}`,
PROFILE: 'a/:accountID',
Copy link
Contributor

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

{ "/": "/a/*", "comment": "Profile Page" }

And to AndroidManifest.xml for Android

<data android:scheme="https" android:host="new.expensify.com" android:pathPrefix="/a"/> 

Copy link
Contributor Author

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.

Copy link
Contributor

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)

getProfileRoute: (accountID) => `a/${accountID}`,
REPORT_PARTICIPANTS: 'r/:reportID/participants',
getReportParticipantsRoute: (reportID) => `r/${reportID}/participants`,
REPORT_PARTICIPANT: 'r/:reportID/participants/details',
getReportParticipantRoute: (reportID, login) => `r/${reportID}/participants/details?login=${encodeURIComponent(login)}`,
REPORT_PARTICIPANT: 'r/:reportID/participants/a/:accountID',
getReportParticipantRoute: (reportID, accountID) => `r/${reportID}/participants/a/${accountID}`,
REPORT_WITH_ID_DETAILS: 'r/:reportID/details',
getReportDetailsRoute: (reportID) => `r/${reportID}/details`,
REPORT_SETTINGS: 'r/:reportID/settings',
Expand Down
3 changes: 2 additions & 1 deletion src/components/BlockingViews/BlockingView.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const propTypes = {
title: PropTypes.string.isRequired,

/** Subtitle message below the title */
subtitle: PropTypes.string.isRequired,
subtitle: PropTypes.string,

/** Link message below the subtitle */
link: PropTypes.string,
Expand All @@ -40,6 +40,7 @@ const propTypes = {

const defaultProps = {
iconColor: themeColors.offline,
subtitle: '',
shouldShowLink: false,
link: 'notFound.goBackHome',
iconWidth: variables.iconSizeSuperLarge,
Expand Down
3 changes: 2 additions & 1 deletion src/components/ReportWelcomeText.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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]))}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was overlooked. participantAccountIDs order was different from participants and it caused regression - Web - New Group - Wrong user details page opens on clicking at display names

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#20583 (comment) for more details about the root cause.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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>
Expand Down
1 change: 1 addition & 0 deletions src/languages/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export default {
and: 'and',
details: 'Details',
privacy: 'Privacy',
hidden: 'Hidden',
delete: 'Delete',
archived: 'archived',
contacts: 'Contacts',
Expand Down
1 change: 1 addition & 0 deletions src/languages/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export default {
and: 'y',
details: 'Detalles',
privacy: 'Privacidad',
hidden: 'Oculto',
delete: 'Eliminar',
archived: 'archivado',
contacts: 'Contactos',
Expand Down
15 changes: 13 additions & 2 deletions src/libs/Navigation/AppNavigator/ModalStackNavigators.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,16 @@ const DetailsModalStackNavigator = createModalStackNavigator([
},
]);

const ProfileModalStackNavigator = createModalStackNavigator([
{
getComponent: () => {
const ProfilePage = require('../../../pages/ProfilePage').default;
return ProfilePage;
},
name: 'Profile_Root',
},
]);

const ReportDetailsModalStackNavigator = createModalStackNavigator([
{
getComponent: () => {
Expand Down Expand Up @@ -223,8 +233,8 @@ const ReportParticipantsModalStackNavigator = createModalStackNavigator([
},
{
getComponent: () => {
const DetailsPage = require('../../../pages/DetailsPage').default;
return DetailsPage;
const ProfilePage = require('../../../pages/ProfilePage').default;
return ProfilePage;
},
name: 'ReportParticipants_Details',
},
Expand Down Expand Up @@ -725,6 +735,7 @@ export {
IOUSendModalStackNavigator,
SplitDetailsModalStackNavigator,
DetailsModalStackNavigator,
ProfileModalStackNavigator,
ReportDetailsModalStackNavigator,
TaskModalStackNavigator,
ReportSettingsModalStackNavigator,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ function RigthModalNavigator() {
options={defaultModalScreenOptions}
component={ModalStackNavigators.DetailsModalStackNavigator}
/>
<Stack.Screen
name="Profile"
options={defaultModalScreenOptions}
component={ModalStackNavigators.ProfileModalStackNavigator}
/>
<Stack.Screen
name="Report_Details"
options={defaultModalScreenOptions}
Expand Down
5 changes: 5 additions & 0 deletions src/libs/Navigation/linkingConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,11 @@ export default {
Details_Root: ROUTES.DETAILS,
},
},
Profile: {
screens: {
Profile_Root: ROUTES.PROFILE,
},
},
Participants: {
screens: {
ReportParticipants_Root: ROUTES.REPORT_PARTICIPANTS,
Expand Down
6 changes: 3 additions & 3 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1057,14 +1057,14 @@ function getReport(reportID) {
* @param {Object} report
*/
function navigateToDetailsPage(report) {
const participants = lodashGet(report, 'participants', []);
const participantAccountIDs = lodashGet(report, 'participantAccountIDs', []);

if (isChatRoom(report) || isPolicyExpenseChat(report) || isThread(report)) {
Navigation.navigate(ROUTES.getReportDetailsRoute(report.reportID));
return;
}
if (participants.length === 1) {
Navigation.navigate(ROUTES.getDetailsRoute(participants[0]));
if (participantAccountIDs.length === 1) {
Navigation.navigate(ROUTES.getProfileRoute(participantAccountIDs[0]));
return;
}
Navigation.navigate(ROUTES.getReportParticipantsRoute(report.reportID));
Expand Down
44 changes: 44 additions & 0 deletions src/libs/actions/PersonalDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding isLoading to PERSONAL_DETAILS_LIST changes personalDetailsList. Lots of components subscribe to this and rerender causing a performance issue. #37986

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
*
Expand Down Expand Up @@ -427,6 +470,7 @@ export {
deleteAvatar,
openMoneyRequestModalPage,
openPersonalDetailsPage,
openPublicProfilePage,
extractFirstAndLastNameFromAvailableDetails,
updateDisplayName,
updateLegalName,
Expand Down
Loading
Loading