[C+ checklist needs completion] [$500] Web - The display name doesn't show after logout and re-visit current page #28077
Comments
kbecciv
added
External
melvin-bot
bot
changed the title
|
Job added to Upwork: https://www.upwork.com/jobs/~01dcb3033fb288e0ce |
|
Triggered auto assignment to @trjExpensify ( |
Bug0 Triage Checklist (Main S/O)
|
melvin-bot
bot
added
the
Help Wanted
|
Triggered auto assignment to @greg-schroeder ( |
|
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Ollyws ( |
|
Proposal by: @suneox ProposalPlease re-state the problem that we are trying to solve in this issue.The display name doesn’t show after logout and re-visit current page What is the root cause of that problem?The user info load when on first time login via /api?command=OpenApp but the input defaultValue will get empty on the first time What changes do you think we should make in order to solve the problem?At withCurrentUserPersonalDetails.js should get loading status with key return withOnyx({
+ isLoading: {
+ key: ONYXKEYS.IS_LOADING_APP,
+ },
personalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
},
session: {
key: ONYXKEYS.SESSION,
},
})(withCurrentUserPersonalDetails);also config propTypes and defaultProp for isLoading ...
+ if(props.isLoading) {
+ return <FullScreenLoadingIndicator />
+ }
return (
...
)A lot of pages use withCurrentUserPersonalDetails so we should add isLoading status to aware using for the next feature or fix the issue with this What alternative solutions did you explore? (Optional)Create withCurrentUserPersonalDetailsAndLoadingState the same withCurrentUserPersonalDetails and same isLoading config from primary solution function WithCurrentUserPersonalDetails(props) {
const accountID = props.session.accountID;
const accountPersonalDetails = props.personalDetails[accountID];
const currentUserPersonalDetails = useMemo(() => ({...accountPersonalDetails, accountID}), [accountPersonalDetails, accountID]);
+ if (props.isLoading) {
+ return <FullScreenLoadingIndicator />;
+ }
return (
<WrappedComponent
...
/>
);
} |
ProposalPlease re-state the problem that we are trying to solve in this issue.display name doesn't show if page is visited via deeplink What is the root cause of that problem?The default value is not present by the time page renders. What changes do you think we should make in order to solve the problem?We can maintain a name state and set this state everytime currentUserDetails changes, the solution would look something like this. const currentUserDetails = props.currentUserPersonalDetails || {};
const [name, setName] = useState({firstName: lodashGet(currentUserDetails, 'firstName', ''), lastName: lodashGet(currentUserDetails, 'lastName', '')});
useEffect(() => {
if (!currentUserDetails) return;
setName({firstName: lodashGet(currentUserDetails, 'firstName', ''), lastName: lodashGet(currentUserDetails, 'lastName', '')});
}, [currentUserDetails]);Now we can use this same state to get values in the component like this <TextInput
inputID="firstName"
name="fname"
label={props.translate('common.firstName')}
accessibilityLabel={props.translate('common.firstName')}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.TEXT}
defaultValue={lodashGet(name, 'firstName', '')}
maxLength={CONST.DISPLAY_NAME.MAX_LENGTH}
spellCheck={false}
/>and <TextInput
inputID="lastName"
name="lname"
label={props.translate('common.lastName')}
accessibilityLabel={props.translate('common.lastName')}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.TEXT}
defaultValue={lodashGet(name, 'lastName', '')}
maxLength={CONST.DISPLAY_NAME.MAX_LENGTH}
spellCheck={false}
/>What alternative solutions did you explore? (Optional)N/A |
ProposalPlease re-state the problem that we are trying to solve in this issue.display name doesn't show if the page is visited via a deep link What is the root cause of that problem?The default value is not present by the time page renders. What changes do you think we should make in order to solve the problem?We can wrap
{currentUserDetails.displayName ? (
<>
<View style={styles.mb4}>
<TextInput
inputID="firstName"
name="fname"
label={props.translate('common.firstName')}
accessibilityLabel={props.translate('common.firstName')}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.TEXT}
defaultValue={lodashGet(currentUserDetails, 'firstName', '')}
maxLength={CONST.DISPLAY_NAME.MAX_LENGTH}
spellCheck={false}
/>
</View>
<View>
<TextInput
inputID="lastName"
name="lname"
label={props.translate('common.lastName')}
accessibilityLabel={props.translate('common.lastName')}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.TEXT}
defaultValue={lodashGet(currentUserDetails, 'lastName', '')}
maxLength={CONST.DISPLAY_NAME.MAX_LENGTH}
spellCheck={false}
/>
</View>
</>
) : <FullscreenLoadingIndicator style={[styles.flex1, styles.pRelative]} />}What alternative solutions did you explore? (Optional)We can use Contributor detailsYour Expensify account email: dubich.sv@gmail.com |
|
📣 @SergeyHTML! 📣
|
Contributor detailsYour Expensify account email: dubich.sv@gmail.com |
|
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
|
Contributor details |
|
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
ProposalPlease re-state the problem that we are trying to solve in this issue.firstName and lastName inputs doesn't rerender with fresh data coming from accountPersonalDetails What is the root cause of that problem?The default value is empty string, because at the first render data is unaccessible. What changes do you think we should make in order to solve the problem?Move |
|
IMPORTANT: ProposalPlease re-state the problem that we are trying to solve in this issueDisplay name fields are empty if accessed via deep link What is the root cause of that problem?The issue arises from the fact that the What changes do you think we should make in order to solve the problem?I recommend adding a Then below the currentUserDetails initialization:App/src/pages/settings/Profile/DisplayNamePage.js Lines 40 to 41 in 70e9fa0 We add this function: const [isLoadingDisplayName, setIsLoadingDisplayName] = useState(true);
useEffect(() => {
setIsLoadingDisplayName(false);
if (currentUserDetails.firstName === undefined || currentUserDetails.lastName === undefined) {
setIsLoadingDisplayName(true);
}
}, [currentUserDetails.lastName, currentUserDetails.firstName]);With that, we guarantee that while the Videofixed-display-name.movHere are my suggested changes for review If my proposal is accepted, I can open a PR in a few hours to fix this. Thank you! What alternative solutions did you explore? (Optional)N/A |
|
Awaiting proposal review @Ollyws |
|
My proposal provided at 21/10 and I would like to update a minor proposal bellow ProposalPlease re-state the problem that we are trying to solve in this issue.The display name doesn't show after logout and re-visit current page What is the root cause of that problem?User info is loaded when the first time login via What changes do you think we should make in order to solve the problem?At withCurrentUserPersonalDetails.js should get loading status with key return withOnyx({
+ isLoading: {
+ key: ONYXKEYS.IS_LOADING_REPORT_DATA, // or ONYXKEYS.IS_LOADING_APP
+ },
personalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
},
session: {
key: ONYXKEYS.SESSION,
},
})(withCurrentUserPersonalDetails);also config propTypes and defaultProp for isLoading ...
+ if(props.isLoading) {
+ return <FullScreenLoadingIndicator />
+ }
return (
...
)or {isLoading ? <FullScreenLoadingIndicator style={[styles.flex1, styles.pRelative]} /> : <FormProvider ... />}A lot of pages use withCurrentUserPersonalDetails so we should add isLoading status into withCurrentUserPersonalDetails.js to be aware of using it for the next feature or fix issues the same with this What alternative solutions did you explore? (Optional)Create withCurrentUserPersonalDetailsAndLoadingState the same withCurrentUserPersonalDetails and the same isLoading config from primary solution function WithCurrentUserPersonalDetails(props) {
const accountID = props.session.accountID;
const accountPersonalDetails = props.personalDetails[accountID];
const currentUserPersonalDetails = useMemo(() => ({...accountPersonalDetails, accountID}), [accountPersonalDetails, accountID]);
+ if (props.isLoading) {
+ return <FullScreenLoadingIndicator />;
+ }
return (
<WrappedComponent
...
/>
);
}We apply this change to make it consistent with |
|
Will review asap. |
|
📣 @suneox 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
|
just waiting on @thienlnam to approve! |
|
Thank @thienlnam and @Ollyws I have accepted an offer and will create PR within today |
melvin-bot
bot
added
Reviewing
melvin-bot
bot
added
Weekly
melvin-bot
bot
changed the title
|
|
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.80-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2023-10-18. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
For reference, here are some details about the assignees on this issue: As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
|
Processing |
greg-schroeder
changed the title
|
Payment complete - just need to do the checklist and we can close this out @Ollyws! |
|
BugZero Checklist: |
|
Thanks @Ollyws. Closing, then. |
|
Thank @Ollyws @greg-schroeder so much I have received payment |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
Display name should show
Actual Result:
Display name is empty
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.73.0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
001.mp4
Recording.4727.mp4
Expensify/Expensify Issue URL:
Issue reported by: @suneox
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695312629364939
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: