Add a tooltip for profile avatar and online status icon #6061

kakajann · 2021-10-26T04:32:39Z

Details

Tooltip added for profile avatar and online status/offline dot

Fixed Issues

$ #5973

Tests

Navgiate to New Expensify
Hover over the avatar profile icon and the online dot

QA Steps

Navigate to New Expensify
Hover over the avatar profile icon and the online dot
Make sure there is a tooltip over the avatar with name of the user
And overing over the online dot, you can see tooltip describing your status

Tested On

Mobile Web
Desktop

Screenshots

mountiny

LGTM so far, just left once comment for now. Great work @kakajann !

mountiny · 2021-10-26T10:25:32Z

src/pages/home/sidebar/SidebarLinks.js

+                            source={this.props.myPersonalDetails.avatar}
+                            isActive={this.props.network && !this.props.network.isOffline}
+                            isSyncing={this.props.network && !this.props.network.isOffline && this.props.isSyncingData}
+                            tooltipText={`${this.props.myPersonalDetails.displayName}`}


Since the myPersonalDetails.displayName is string, is there any particular reason why you pass it inside the `` characters? 🤔

Whoops, I'll fix it soon

mountiny · 2021-10-26T10:27:35Z

src/languages/es.js

+        online: 'Online',
+        offline: 'Offline',
+        syncing: 'Syncing',


I see you have asked for the translations in Slack, well done. I have also asked internally, if we even want to translate the online/offline, since they are so common.

Only changed syncing to Sincronizando

mountiny

@kakajann Thank you very much. I have added the other two translations which came up from the translation convo (there is another one which is internal and has more exposure, so I have pinged it there).

src/languages/es.js

Co-authored-by: Vit Horacek <[email protected]>

mountiny

@kakajann Great work, I think I found just one last change to request, thank you for addressing it and then we can merge!

src/components/AvatarWithIndicator.js

Co-authored-by: Vit Horacek <[email protected]>

parasharrajat · 2021-10-28T02:55:07Z

@mountiny I don't think showing the user his own name for Avatar is a good idea. It should show what it will do. So it should be Settings same as before.

mountiny

Commented in the code. Thank you!

mountiny · 2021-10-28T08:21:12Z

src/pages/settings/InitialPage.js

@@ -181,6 +181,7 @@ const InitialSettingsPage = ({
                                size="large"
                                source={myPersonalDetails.avatar}
                                isActive={network.isOffline === false}
+                                tooltipText={myPersonalDetails.displayName}


@parasharrajat that is a very good point and you are right. I have overlooked this 🤦 Thank you 🙇

@kakajann could you please change this copy to say settings. That should be last change to this PR I believe.

kakajann · 2021-10-28T09:09:15Z

Issues expected result was:

There should be a tooltip showing the following:

For the avatar - The full name of the user

For the online dot - The online/offline status

I've also checked the slack thread. Here's @shawnborton 's comment:

I think the tooltip would just say your name or primary login when you hover over the picture, and a separate one could say offline/online when hovered over the small status circle

Also what Slack does:

@mountiny are you sure you want it to be Settings rather than user fullname?

mountiny · 2021-10-28T09:18:30Z

@kakajann thanks for the breakdown. I think the question is accessibility now. It is the button which gets you to Settings and having the tooltip tell you that would be useful.

Le's just double check with Shawn and others. Sorry for the delays, just better to clarify it now, than later 🙇‍♂️

mountiny · 2021-10-28T11:11:28Z

@kakajann Asked in the original thread here.

mountiny · 2021-11-01T10:39:34Z

Well, there was not many opinions shared in the thread, but we will keep it as it is here and was originally in the issue description! @kakajann Thank you for your patience 🙇

mountiny

Great work on this one! Thank you for it 🙌

OSBotify · 2021-11-01T11:59:25Z

🚀 Deployed to staging by @mountiny in version: 1.1.11-5 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2021-11-02T23:36:36Z

🚀 Deployed to production by @yuwenmemon in version: 1.1.12-3 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

add tooltip for user avatar and status

4c72058

kakajann requested a review from a team as a code owner October 26, 2021 04:32

MelvinBot requested review from mountiny and removed request for a team October 26, 2021 04:32

kakajann added 2 commits October 26, 2021 09:35

resolve conflicts

aa69d3c

remove unused Tooltip import

9ba543a

mountiny reviewed Oct 26, 2021

View reviewed changes

kakajann added 3 commits October 26, 2021 19:14

fix minor code styling

bc7787a

change spanish translation for syncing

e566d0f

trigger github actions

30546dc

mountiny self-requested a review October 27, 2021 17:44

mountiny requested changes Oct 27, 2021

View reviewed changes

src/languages/es.js Outdated Show resolved Hide resolved

update Spanish translations

be02ee5

Co-authored-by: Vit Horacek <[email protected]>

mountiny requested changes Oct 27, 2021

View reviewed changes

src/components/AvatarWithIndicator.js Outdated Show resolved Hide resolved

update JSDoc of userStatus function

899d640

Co-authored-by: Vit Horacek <[email protected]>

mountiny approved these changes Oct 27, 2021

View reviewed changes

mountiny requested changes Oct 28, 2021

View reviewed changes

mountiny self-requested a review November 1, 2021 10:39

mountiny approved these changes Nov 1, 2021

View reviewed changes

mountiny merged commit 0c44613 into Expensify:main Nov 1, 2021

OSBotify mentioned this pull request Nov 1, 2021

Deploy Checklist: New Expensify 2021-10-29 #6127

Closed

23 tasks

botify mentioned this pull request Nov 2, 2021

[HOLD for payment 2021-11-09] LHN - Add a tooltip for profile avatar and online status icon - Reported by: @Santhosh-Sellavel #5973

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a tooltip for profile avatar and online status icon #6061

Add a tooltip for profile avatar and online status icon #6061

kakajann commented Oct 26, 2021 •

edited by mountiny

Loading

mountiny left a comment

mountiny Oct 26, 2021

kakajann Oct 26, 2021

mountiny Oct 26, 2021

kakajann Oct 26, 2021

mountiny left a comment

mountiny left a comment

parasharrajat commented Oct 28, 2021

mountiny left a comment

mountiny Oct 28, 2021

kakajann commented Oct 28, 2021

mountiny commented Oct 28, 2021

mountiny commented Oct 28, 2021

mountiny commented Nov 1, 2021

mountiny left a comment

OSBotify commented Nov 1, 2021

OSBotify commented Nov 2, 2021

Add a tooltip for profile avatar and online status icon #6061

Add a tooltip for profile avatar and online status icon #6061

Conversation

kakajann commented Oct 26, 2021 • edited by mountiny Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

mountiny left a comment

Choose a reason for hiding this comment

mountiny Oct 26, 2021

Choose a reason for hiding this comment

kakajann Oct 26, 2021

Choose a reason for hiding this comment

mountiny Oct 26, 2021

Choose a reason for hiding this comment

kakajann Oct 26, 2021

Choose a reason for hiding this comment

mountiny left a comment

Choose a reason for hiding this comment

mountiny left a comment

Choose a reason for hiding this comment

parasharrajat commented Oct 28, 2021

mountiny left a comment

Choose a reason for hiding this comment

mountiny Oct 28, 2021

Choose a reason for hiding this comment

kakajann commented Oct 28, 2021

mountiny commented Oct 28, 2021

mountiny commented Oct 28, 2021

mountiny commented Nov 1, 2021

mountiny left a comment

Choose a reason for hiding this comment

OSBotify commented Nov 1, 2021

OSBotify commented Nov 2, 2021

kakajann commented Oct 26, 2021 •

edited by mountiny

Loading