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

Added tooltips to all Icon buttons #4661

Merged
merged 12 commits into from
Aug 24, 2021

Conversation

mananjadhav
Copy link
Collaborator

@mananjadhav mananjadhav commented Aug 14, 2021

@iwiznia Can you please review this?

Details

Wrapped all TouchableOpacity + Icon components with Tooltip.

Fixed Issues

$ #4499

Tests

  1. Verified tooltips for IOUModal, Search, ReportActionCompose, AttachmentView, VideoChat.
  2. Verified Tooltips for Web and Desktop
  3. Tested for no impact on Mobile Web, iOS and Android

QA Steps

  1. Need to verify if all the icon buttons are covered. (exclude Green FAB)
  2. Need to verify if any tooltip has any alignment issue.
  3. Added tooltips for icon buttons in IOUModal, Search, ReportActionCompose, AttachmentView, VideoChat.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

tooltip-icon-close-download png

tooltip-icon-close-web

tooltip-icon-emoji-web

tooltip-icon-pin-web

tooltip-icon-search-web

tooltip-icon-unpin-web

tooltip-icon-vc-web

Mobile Web

NA

Desktop

tooltip-icon-close-desktop

tooltip-icon-download-desktop

tooltip-icon-emoji-desktop

tooltip-icon-pin-desktop

tooltip-icon-search-desktop

tooltip-icon-unpin-desktop

tooltip-icon-vc-desktop

iOS

NA

Android

NA

@mananjadhav mananjadhav requested a review from a team as a code owner August 14, 2021 10:01
@MelvinBot MelvinBot requested review from aldo-expensify and removed request for a team August 14, 2021 10:01
@mananjadhav
Copy link
Collaborator Author

@iwiznia PR raised.

Need help:

  1. to confirm coverage. I'd search all Icon across the codebase and updated all relevant pressable icon
  2. Translations in Spanish.
  3. I haven't added Tooltips to the following buttons:
    a. Main Send button in the Chat Input Box
    b. Green FAB on the LHN
    c. + button that opens up "Add attachment" option

Let me know if you feel tooltips are required for those too.

@@ -36,6 +36,10 @@ export default {
contacts: 'Contactos',
recents: 'Recientes',
close: 'Cerrar',
download: 'Descargar',
pin: 'Alfiler',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pin: 'Alfiler',
pin: 'Anclar',

download: 'Descargar',
pin: 'Alfiler',
unPin: 'Desprender',
back: 'De vuelta',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
back: 'De vuelta',
back: 'Volver',

@@ -36,6 +36,10 @@ export default {
contacts: 'Contactos',
recents: 'Recientes',
close: 'Cerrar',
download: 'Descargar',
pin: 'Alfiler',
unPin: 'Desprender',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
unPin: 'Desprender',
unPin: 'Desanclar',

@@ -101,6 +105,7 @@ export default {
nameEmailOrPhoneNumber: 'Nombre, email o número de teléfono',
},
videoChatButtonAndMenu: {
tooltip: 'Chat de video',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
tooltip: 'Chat de video',
tooltip: 'Videollamada',

@iwiznia
Copy link
Contributor

iwiznia commented Aug 16, 2021

I haven't added Tooltips to the following buttons:

Why not?

@iwiznia
Copy link
Contributor

iwiznia commented Aug 16, 2021

Oh, also can you make sure to test in the other platforms? Even if it is to check there are no regressions and buttons look/work properly

@mananjadhav
Copy link
Collaborator Author

I'll update the translation changes.

Why not?
One of them was a primary action. Other is a menu. I can add. Can you help with the translations for these two buttons ?
image

Oh, also can you make sure to test in the other platforms? Even if it is to check there are no regressions and buttons look/work properly
I've checked the other platforms - Mobile Web, iOS, and Android.

@iwiznia
Copy link
Contributor

iwiznia commented Aug 16, 2021

One of them was a primary action. Other is a menu. I can add. Can you help with the translations for these two buttons ?

Sure. Let me tag the @Expensify/marketing team so they can check the copy (all new copy is in this file, they are the green lines) and give us copy for the tooltips of these buttons:
a. Main Send button in the Chat Input Box
b. Green FAB on the LHN
c. + button that opens up "Add attachment" option

I can then translate the new ones for you.

@iwiznia iwiznia added the Waiting for copy User facing verbiage needs polishing label Aug 16, 2021
@mananjadhav
Copy link
Collaborator Author

@iwiznia One quick question, what are your thoughts on opening the FAB menu and the + icon menu on hover? rather than adding Tooltip?

@iwiznia
Copy link
Contributor

iwiznia commented Aug 16, 2021

I am personally not quite fond of things that open on hover. In any case that's just one opinion, you can propose and discuss changes like that one in the slack channel.

@mananjadhav
Copy link
Collaborator Author

PR Updated with suggested translations and tooltip for send button.

@iwiznia
Copy link
Contributor

iwiznia commented Aug 16, 2021

b. Green FAB on the LHN
c. + button that opens up "Add attachment" option

We are still waiting for those, right?

@mananjadhav
Copy link
Collaborator Author

Yes waiting.

@@ -36,6 +36,10 @@ export default {
contacts: 'Contactos',
recents: 'Recientes',
close: 'Cerrar',
download: 'Descargar',
pin: 'Anclar',
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest using Fijar. This is the terminology used in popular apps like WhatsApp and Twitter.

@@ -36,6 +36,10 @@ export default {
contacts: 'Contactos',
recents: 'Recientes',
close: 'Cerrar',
download: 'Descargar',
pin: 'Anclar',
unPin: 'Desanclar',
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest using Desfijar. This is the terminology used in popular apps like WhatsApp and Twitter.

@mananjadhav
Copy link
Collaborator Author

@marklouisdeshaun Thanks, Mark. Will update. Can you help with the copy of the other two buttons? Floating Action Button in the sidebar and + button to open Add Attachments in the chat?

Screenshot 2021-08-16 at 5 25 56 PM

Copy link
Contributor

@aldo-expensify aldo-expensify left a comment

Choose a reason for hiding this comment

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

I tested this locally and seemed to work fine.
I agree with the proposed changes on the spanish translations.

@mananjadhav
Copy link
Collaborator Author

@iwiznia @marklouisdeshaun Left with the copy & translation for the + action on ReportActionCompose. Can you help with that to close it at the earliest?

iwiznia
iwiznia previously approved these changes Aug 17, 2021
@mananjadhav
Copy link
Collaborator Author

I've set the labels as Actions and Acción right now.

iwiznia
iwiznia previously approved these changes Aug 17, 2021
@mananjadhav
Copy link
Collaborator Author

@iwiznia @marklouisdeshaun Left with the copy & translation for the + action on ReportActionCompose. Can you help with that to close it at the earliest?

@marklouisdeshaun Can you help me with the copy of this button? So that this can be moved to closure?

…ooltip-icon-button

# Conflicts:
#	src/components/HeaderWithCloseButton.js
#	src/components/VideoChatButtonAndMenu.js
@aldo-expensify aldo-expensify added Waiting for copy User facing verbiage needs polishing and removed Waiting for copy User facing verbiage needs polishing labels Aug 19, 2021
@iwiznia iwiznia merged commit ab18ee8 into Expensify:main Aug 24, 2021
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@isagoico
Copy link

Hello @mananjadhav, any QA tests needed for this PR?

@mananjadhav
Copy link
Collaborator Author

@isagoico We've added tooltips to all icon buttons. While I've added checking all the codebase. Let me know if I've missed any. In addition to this, we might have to test the alignment. While I've added content to QA steps, I am not sure the best way to describe step for each icon button.

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.0.88-2 🚀

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

@botify
Copy link

botify commented Sep 1, 2021

This has been deployed to production and is now subject to a 7-day regression period.
If no regressions arise, payment will be issued on 2021-09-08. 🎊

@eVoloshchak
Copy link
Contributor

Leaving a note here that the PR has caused a regression: #14043
The 'close' and 'download' tooltips are slightly misaligned
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Waiting for copy User facing verbiage needs polishing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants