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

Show a user's display name for 'is typing' indicator #2317

Merged
merged 3 commits into from
Apr 13, 2021

Conversation

Jag96
Copy link
Contributor

@Jag96 Jag96 commented Apr 8, 2021

@NikkiWines will you please review this?

Details

This PR adds a check for the displayName inside getDisplayName, since we can set that value inside formatPersonalDetails but not set the first/last name. I've also added the first/last name to formatPersonalDetails to ensure that when we fetch the personalDetailsList, we're storing those values in Onyx. If there is a reason we don't want to do this, please let me know, but since we're using first/last name in other parts of the app it made sense to add it here.

Fixed Issues

Fixes Expensify/Expensify/issues/159835

Tests

  1. Login as User A who has personal details set for first and last name (you can set these values by clicking on the profile icon, then clicking on profile)
  2. Login as User B and start a chat with User A
  3. Type in the textbox as User A, see that User A's first and last name show up instead of their email address
  4. For a user who does not have first/last name set, type as that user and confirm that in the opposite chat window the email address still shows

I also confirmed the following works

  1. User A types, User B sees personalDetail name
  2. User A changes name, User A sees personalDetail name updated everywhere
  3. User A types, User B sees old name in "is typing" indicator
  4. User B refreshes
  5. User A types after refresh, "Is typing" indicator shows User A's new name

QA Steps

  1. Login as User A who has personal details set for first and last name (you can set these values by clicking on the profile icon, then clicking on profile)
  2. Login as User B and start a chat with User A
  3. Type in the textbox as User A, see that User A's first and last name show up instead of their email address

Note: a regression is being added for this in https://github.com/Expensify/Expensify/issues/159794

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

image
image

@Jag96 Jag96 requested a review from NikkiWines April 8, 2021 23:28
@Jag96 Jag96 requested a review from a team as a code owner April 8, 2021 23:28
@Jag96 Jag96 self-assigned this Apr 8, 2021
@MelvinBot MelvinBot requested review from MariaHCD and removed request for a team April 8, 2021 23:29
MariaHCD
MariaHCD previously approved these changes Apr 9, 2021
Copy link
Contributor

@NikkiWines NikkiWines left a comment

Choose a reason for hiding this comment

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

Looks good, one comment about the re-introduction of userDetails.displayName

@Jag96
Copy link
Contributor Author

Jag96 commented Apr 9, 2021

OK, #2317 (comment) is sorted, this is ready for another review

Copy link
Contributor

@NikkiWines NikkiWines left a comment

Choose a reason for hiding this comment

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

Works well!!

@Jag96
Copy link
Contributor Author

Jag96 commented Apr 10, 2021

Updated!

Copy link
Contributor

@NikkiWines NikkiWines left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Contributor

@MariaHCD MariaHCD left a comment

Choose a reason for hiding this comment

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

LGTM!

@MariaHCD MariaHCD merged commit 9b09384 into master Apr 13, 2021
@MariaHCD MariaHCD deleted the joe-fix-displayName-typing branch April 13, 2021 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants