Skip to content

Conversation

@RonLek
Copy link
Contributor

@RonLek RonLek commented Dec 22, 2020

Proposed changes (including videos or screenshots)

Previously the header icons in the sidebar didn't show a tooltip when hovered over. This PR fixes that.

Screenshot from 2020-12-22 15-17-41

Issue(s)

Closes #19908

@RonLek
Copy link
Contributor Author

RonLek commented Dec 22, 2020

@dougfabris do you think it would be a good idea to replace the sort icon with something more intuitive since it represents more than just sort (for example, view modes and group by)? Would love to hear your thoughts on this.

Copy link
Member

@dougfabris dougfabris left a comment

Choose a reason for hiding this comment

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

@dougfabris do you think it would be a good idea to replace the sort icon with something more intuitive since it represents more than just sort (for example, view modes and group by)? Would love to hear your thoughts on this.

Well, for now, we shouldn't replace the icon. Just replace the title for Filters, I think it can be more descriptive.

@RonLek RonLek force-pushed the sidenav_header_tooltip branch from 0c5468b to 139d590 Compare December 22, 2020 14:24
@RonLek
Copy link
Contributor Author

RonLek commented Dec 22, 2020

Thanks! @dougfabris. Made the changes.

@dougfabris
Copy link
Member

@RonLek Thank you for helping us =)

@dougfabris dougfabris requested a review from ggazzo December 22, 2020 17:11
@dougfabris dougfabris added this to the 3.10.0 milestone Dec 22, 2020
dougfabris
dougfabris previously approved these changes Dec 22, 2020
Copy link
Member

@ggazzo ggazzo left a comment

Choose a reason for hiding this comment

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

hey thanks for your time, can I ask you to use translations instead of the raw text?

look for useTranslation

@ggazzo ggazzo modified the milestones: 3.10.0, 3.11.0 Dec 22, 2020
@RonLek
Copy link
Contributor Author

RonLek commented Dec 23, 2020

@ggazzo Definitely! For the ones not already having translations in languages other than English should I add them in this PR itself or raise a different one similar to this?

@dougfabris
Copy link
Member

dougfabris commented Dec 25, 2020

@RonLek as these translations are related to the fix I think it's not a problem to add in this PR.

@RonLek
Copy link
Contributor Author

RonLek commented Dec 27, 2020

Thanks! @dougfabris . Added translations in this PR itself : )

@RonLek RonLek requested review from dougfabris and ggazzo December 27, 2020 20:00
@RonLek
Copy link
Contributor Author

RonLek commented Jan 4, 2021

@dougfabris @ggazzo do let me know if any changes required.

Even though the translations are double-checked should translations be done only in English for the time being in this PR since we're taking in native speakers' contributions for translations?

Awaiting your views on this one : )

@ggazzo
Copy link
Member

ggazzo commented Jan 12, 2021

@RonLek I would ask you to do not create one more key, please use Create_A_New_Channel instead of Create_Room

@RonLek RonLek force-pushed the sidenav_header_tooltip branch from e88fda2 to 66ab999 Compare January 12, 2021 15:16
Lint fix

Adds tooltip for sidebar header icons

Modifies Sort and CreateRoom titles

Adds translations for sidenav header icons

Fixes lint

Uses translations for sidenav header icons

Uses translations for sidenav header icons

Uses Create_A_New_Channel instead of creating Create_Room

Rebases with develop
@RonLek RonLek force-pushed the sidenav_header_tooltip branch from 0a6a642 to c2056cd Compare January 12, 2021 17:46
@RonLek
Copy link
Contributor Author

RonLek commented Jan 12, 2021

Thanks @ggazzo. I've made the changes. Please review.

@RonLek
Copy link
Contributor Author

RonLek commented Jan 26, 2021

@dougfabris @ggazzo please take a look :)

@ggazzo
Copy link
Member

ggazzo commented Feb 9, 2021

@RonLek can I ask you from where did you get the translations? I'm not very fluent in some of them :p just to be sure that makes sense

@RonLek
Copy link
Contributor Author

RonLek commented Feb 9, 2021

@ggazzo Definitely! All of the translations have been taken up from Google Translate after a double-check.

What I mean by a double-check is that if "Home" translates to "Casa" in Spanish, then I've reverse translated "Casa" to English and seen if that checks out back to "Home" and only then inserted the translation for the word (skipped in cases it didn't). The reason for this is that I'm aware Google Translate might not capture the context of the word when just a single word is entered for translation but doing a reverse-check confirms if the translation was correct.

Let me know what you think : )

@ggazzo
Copy link
Member

ggazzo commented Feb 9, 2021

Ok, casa in portuguese means home as well but still I'm not sure if is a common usage for that word... but no problem thanks

@ggazzo ggazzo merged commit 2e9dbe6 into RocketChat:develop Feb 9, 2021
@sampaiodiego sampaiodiego mentioned this pull request Feb 28, 2021
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.

Tooltip for SideNav Header Icons

3 participants