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 button tooltips #3286

Merged
merged 9 commits into from
Mar 2, 2024
Merged

Added button tooltips #3286

merged 9 commits into from
Mar 2, 2024

Conversation

tanayaxsharma
Copy link
Collaborator

@tanayaxsharma tanayaxsharma commented Jul 7, 2023

Resolves #227

Adds tooltips to skip, stuck, jump, and feedback buttons on both the validate and explore pages. For the skip, stuck, and jump buttons, the messages.en, validation/explore.scala.html files were utilized. For the feedback button, common.json and the corresponding Main.js files under SVLabel and SVValidate were used.

Before/After screenshots

image
image
image
image

@misaugstad misaugstad self-requested a review July 10, 2023 22:30
@tanayaxsharma tanayaxsharma marked this pull request as draft July 11, 2023 23:49
@tanayaxsharma tanayaxsharma marked this pull request as ready for review July 14, 2023 20:08
Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

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

Sorry for taking so long to review this one!

  1. Could you change the style of the tooltips on the Validate page to match those on the Explore page? Sorry, I didn't realize that we already had a couple with a different style! And to be clear: this is just for the buttons on the top and left of the GSV window.
  2. Could you add translations to other languages using Google Translate? Apologies if I told you that I would do this part before 😅 I did write up some better instructions for it here

@@ -63,7 +63,7 @@
<img src='@routes.Assets.at("javascripts/SVLabel/img/icons/comment.png")' alt="Comment icon" align="">
<br>
@Html(Messages("feedback"))
</div>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove the accidental whitespace?

@tanayaxsharma
Copy link
Collaborator Author

I changed the formatting of the tooltips for the skip and feedback buttons and deleted the addition white space in one of the files. Here are some updated screenshots:

Skip Button:
image

Feedback Button:
image

@tanayaxsharma
Copy link
Collaborator Author

Also, added language translations!

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

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

I started testing and noticed that the Stuck button is messed up in Spanish. Make sure to test all the buttons in each language, it doesn't take too long! 😉
Screenshot from 2023-08-14 15-08-34

I think that since you started working on this, we've added German translations (de). Could you add German translations as well?

@@ -241,6 +240,11 @@ function Main (params) {
});
}
});
$("#feedback-button-tooltip").tooltip({
Copy link
Member

Choose a reason for hiding this comment

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

Why doesn't the feedback tooltip work like the other ones, and needs it's own call here? Why can't it be included in the $('[data-toggle="tooltip"]').tooltip({ bit below?

@tanayaxsharma
Copy link
Collaborator Author

I have fixed the tooltip issues in different languages and added translations for German as well.

After attempting to remove the call to the feedback tooltip and trying to include it in the general call below that, I think that the reason it needs its own call is because the text associated with it lies within common.json rather than the messages files. Within the call itself, if I remove title: i18next.t('common:left-ui-feedback'), the tooltip will not appear.

Would you like me to move the text from common.json to the messages files? I believe there will then be both audit.left.ui.feedback = "Share your thoughts with us" and a validate.left.ui.feedback = "Share your thoughts with us".

@misaugstad
Copy link
Member

Would you like me to move the text from common.json to the messages files?

Yeah let's do that, let's try and keep it consistent between the different tooltips.

I'm wondering if part of the issue is that the feedback tooltip looks like it's set up differently from the other ones. The others look like this:

<div id="left-column-stuck-button" class="button" title="@Messages("audit.left.ui.stuck.tooltip")" data-toggle="tooltip" data-placement="top">

the feedback one looks like

<div id="feedback-button-tooltip" data-toggle="tooltip" data-placement="top">

But maybe it was done this way because there's a popover that shows up briefly when feedback is submitted? Might need a bit more testing.

I believe there will then be both audit.left.ui.feedback = "Share your thoughts with us" and a validate.left.ui.feedback = "Share your thoughts with us"

We generally put translations that are shared on multiple pages near the top of the message file. You could probably put it right after this line:

feedback = Feedback

And the last thing is to pull in changes from develop and resolve merge conflicts. Thank you @tanayaxsharma !!

@tanayaxsharma
Copy link
Collaborator Author

I have changed the code so that the text associated with the feedback button's tooltip is now in the messages files instead of the common.json files.

I believe that even though the feedback tooltip is set up differently, it is still functioning correctly after changing just the text's location.

Let me know if you would like me to make any additional changes.

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

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

Thanks @tanayaxsharma looks good!

@misaugstad misaugstad merged commit 6b79288 into develop Mar 2, 2024
@misaugstad misaugstad deleted the 227-adds-tooltips-to-buttons branch March 2, 2024 00:33
@misaugstad misaugstad mentioned this pull request Mar 5, 2024
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.

Tutorial: Give info on hovering over buttons
2 participants