AO3-7323 Update introductory text on logged-out homepage#5629
Conversation
sarken
left a comment
sarken
left a comment
There was a problem hiding this comment.
Hi, nghz!
Thank you so much for this pull request. I took a quick look at it and the changes themselves look good, although the failing test in features/other_a/homepage.feature: need to be updated to look for the next text. (Since this is your first pull request and the issue is meant to be low effort, it's up to you whether or not you'd like to try updating the i18n methods as Reviewdog requests.)
For now, I've updated the Jira issue status to In Review so no one mistakenly creates a duplicate pull request. If you'd like the ability to comment on, assign, and transition issues in the future, you're welcome to create a Jira account! It makes things a bit easier for us on the organizational side if the Full Name on your Jira account either closely matches the name you'd like us to credit in the release notes or includes it in parentheses, e.g. "Nickname (CREDIT NAME)."
Once you've done that (or if you've already done it -- Jira has been unreliable about showing us new accounts in the admin panel lately), you can either reply here or send an email to otw-coders@transformativeworks.org with your account name and email address and we'll set up the permissions for you.
Thanks again for contributing! If you have any questions, you can contact us at the same email address listed above.
github-actions
bot
added
Coder Has Actioned Review
and removed
Reviewed: Action Needed
labels
Hi! Thanks a lot for taking a look at the PR and for the feedback :) I've now updated the failing test in features/other_a/homepage.feature to match the new text. I also updated the ts helpers to t and added the corresponding i18n keys as suggested. I've also created a Jira account now. Please let me know if there's anything else I should adjust! |
LateNghtz
force-pushed
the
AO3-7323/Update-introductory-text-on-homepage
branch
4 times, most recently
from
3b036bc to
37f70b8
Compare
…d parent_org_name link text
LateNghtz
force-pushed
the
AO3-7323/Update-introductory-text-on-homepage
branch
from
37f70b8 to
607d48c
Compare
sarken
left a comment
sarken
left a comment
There was a problem hiding this comment.
Just one more thing about the code -- sorry for not linking to our i18n Standards in my previous comment!
Regarding Jira, we've been having some trouble with the admin panel not updating, and I'm afraid I can't find your account. Can you either reply here or send an email to otw-coders@transformativeworks.org with your account name and email address so we can fix it?
Thanks!
| <h2 class="heading"><%= t(".intro_description") %></h2> | ||
| <p class="stats"><%= ts("more than %{fandom_count} fandoms | %{user_count} users | %{work_count} works", fandom_count: content_tag(:span, number_with_delimiter(@fandom_count), class: "count"), user_count: content_tag(:span, number_with_delimiter(@user_count), class: "count"), work_count: content_tag(:span, number_with_delimiter(@work_count), class: "count")).html_safe %></p> | ||
| <p class="parent"><%= ts("The Archive of Our Own is a project of the") %> <%= link_to ts("Organization for Transformative Works"), "http://transformativeworks.org" %>.</p> | ||
| <p class="parent"><%= t(".parent_org_intro") %> <%= link_to t(".parent_org_name"), "http://transformativeworks.org" %>.</p> |
There was a problem hiding this comment.
To make things easier for our translators, we prefer interpolation over separate strings. Could you update this to use our standards for hyperlinks in translations? (You'll need to move the . before the </p> tag into the locale file as well.)
github-actions
bot
added
Coder Has Actioned Review
and removed
Reviewed: Action Needed
labels
LateNghtz
force-pushed
the
AO3-7323/Update-introductory-text-on-homepage
branch
from
ee75d5e to
ce2eaec
Compare
| <h2 class="heading"><%= t(".intro_description") %></h2> | ||
| <p class="stats"><%= ts("more than %{fandom_count} fandoms | %{user_count} users | %{work_count} works", fandom_count: content_tag(:span, number_with_delimiter(@fandom_count), class: "count"), user_count: content_tag(:span, number_with_delimiter(@user_count), class: "count"), work_count: content_tag(:span, number_with_delimiter(@work_count), class: "count")).html_safe %></p> | ||
| <p class="parent"><%= ts("The Archive of Our Own is a project of the") %> <%= link_to ts("Organization for Transformative Works"), "http://transformativeworks.org" %>.</p> | ||
| <p class="parent"><%= t(".parent_org_intro_html", parent_org_link: link_to(t(".parent_org_name"), "http://transformativeworks.org")) %></p> |
There was a problem hiding this comment.
It looks like the names don't quite line up with our guidelines. It should be either
parent_org_linkfor the variable representing the hyperlink andparent_orgfor the text used in the link, orparent_org_name_linkfor the hyperlink's variable andparent_org_namefor the text.
I'd recommend the first one since it's shorter.
No problem, and thank you for assisting me with my first PR! |
3 participants
Pull Request Checklist
as the first thing in your pull request title (e.g.
AO3-1234 Fix thing)until they are reviewed and merged before creating new pull requests.
Issue
https://otwarchive.atlassian.net/browse/AO3-7323
Purpose
Update introductory text on logged-out homepage to:
"A non-profit, non-commercial archive for transformative fanworks; created by and for fans of books, music, art, games, shows, movies, real-person fiction (RPF), and other fandoms."
and:
adds “(AO3)” to the text underneath the statistics line
Credit
He/Him: nghz