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

Jump to bottom

Translate some texts from Dashboard Page #14963

Merged
merged 8 commits into from Oct 8, 2021
Merged

Translate some texts from Dashboard Page #14963

merged 8 commits into from Oct 8, 2021

Conversation

berviantoleo
Copy link
Contributor

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Translate some texts from Dashboard Page

Related Tickets & Documents

#14888

https://dev.to/devteam/forem-hacktoberfest-let-s-internationalize-404n

QA Instructions, Screenshots, Recordings

Added/updated tests?

  • Yes
  • No, and this is why: existing regression tests should suffice
  • I need help with writing tests

[optional] What gif best describes this PR or how it makes you feel?

Excited GIF

@berviantoleo berviantoleo requested a review from a team as a code owner October 5, 2021 21:43
@pr-triage pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Oct 5, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2021

Thank you for opening this PR! We appreciate you!

For all pull requests coming from third-party forks we will need to
review the PR before we can process it through our CI pipelines.

A Forem Team member will review this contribution and get back to
you as soon as possible!

@berviantoleo berviantoleo mentioned this pull request Oct 5, 2021
Closed
8 tasks
@msarit
Copy link
Contributor

msarit commented Oct 7, 2021

@berviantoleo Hey there! Please resolve conflicts! 🙏🏾

@berviantoleo
Copy link
Contributor Author

Sure.

rhymes
rhymes reviewed Oct 7, 2021
View reviewed changes
Copy link
Contributor

@rhymes rhymes left a comment

Choose a reason for hiding this comment

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

Left a note about the usage of pluralize, thank you @berviantoleo !

app/views/dashboards/_actions.html.erb Outdated
@@ -4,7 +4,7 @@
<a class="crayons-link crayons-link--block <%= "crayons-link--current" if params[:action] == "show" && (params[:which] == "organization" || params[:which].blank?) %>"
href="<%= dashboard_path %>"
<%= params[:action] == "show" && (params[:which] == "organization" || params[:which].blank?) ? ' aria-current="page"'.html_safe : "" %>>
Posts
<%= t("core.post").pluralize %>
Copy link
Contributor

Choose a reason for hiding this comment

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

You could take a look at https://guides.rubyonrails.org/i18n.html#pluralization for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @rhymes , Thank you.

May you elaborate more about this. I'm new at Ruby. Do you mean to use count?

Example:
t("core.post", count: @user.articles_count)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I follow this thing.

https://github.com/forem/forem/blob/main/app/views/notifications/_nav_menu.html.erb#L10

But if you mean to consider the count too. I will update it.

Copy link
Contributor

Choose a reason for hiding this comment

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

In this specific case I would just use core.posts and keep the translation pluralized. If ever we ever need counts we're better off adding a separate key. This is what we settled on for tags as well.

app/views/dashboards/_actions.html.erb Outdated
@@ -22,7 +22,7 @@
<a class="crayons-link crayons-link--block <%= "crayons-link--current" if params[:action] == "followers" %>"
href="/dashboard/user_followers"
<%= params[:action] == "followers" ? ' aria-current="page"'.html_safe : "" %>>
Followers
<%= t("core.follower").pluralize %>
Copy link
Contributor

Choose a reason for hiding this comment

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

same about not using pluralize directly

@maestromac maestromac removed the request for review from a team October 7, 2021 20:02
citizen428
citizen428 previously approved these changes Oct 8, 2021
View reviewed changes
app/views/dashboards/_actions.html.erb Outdated
@@ -4,7 +4,7 @@
<a class="crayons-link crayons-link--block <%= "crayons-link--current" if params[:action] == "show" && (params[:which] == "organization" || params[:which].blank?) %>"
href="<%= dashboard_path %>"
<%= params[:action] == "show" && (params[:which] == "organization" || params[:which].blank?) ? ' aria-current="page"'.html_safe : "" %>>
Posts
<%= t("core.post").pluralize %>
Copy link
Contributor

Choose a reason for hiding this comment

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

In this specific case I would just use core.posts and keep the translation pluralized. If ever we ever need counts we're better off adding a separate key. This is what we settled on for tags as well.

@pr-triage pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Oct 8, 2021
@citizen428 citizen428 dismissed their stale review October 8, 2021 02:59

Approved by accident

@pr-triage pr-triage bot added PR: unreviewed bot applied label for PR's with no review and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Oct 8, 2021
@pr-triage pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Oct 8, 2021
rhymes
rhymes approved these changes Oct 8, 2021
View reviewed changes
Copy link
Contributor

@rhymes rhymes left a comment

Choose a reason for hiding this comment

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

Thank you @berviantoleo!

@pr-triage pr-triage bot added PR: reviewed-approved bot applied label for PR's where reviewer approves changes and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Oct 8, 2021
@rhymes rhymes merged commit acbda16 into forem:main Oct 8, 2021
@pr-triage pr-triage bot removed the PR: reviewed-approved bot applied label for PR's where reviewer approves changes label Oct 8, 2021
@pr-triage pr-triage bot added the PR: merged bot applied label for PR's that are merged label Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@citizen428 citizen428 citizen428 approved these changes

@rhymes rhymes rhymes approved these changes

Assignees
No one assigned
Labels
PR: merged bot applied label for PR's that are merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@berviantoleo @msarit @citizen428 @rhymes