I18nize articles & social_previews #15097

yheuhtozr · 2021-10-18T03:13:49Z

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

Description

Extracts strings for i18n from the most of remaining app/views/articles and related templates. Attached fr locales for testing purposes. Existing translations up to #15055 reflected (hopefully).

Related Tickets & Documents

#14888

QA Instructions, Screenshots, Recordings

Please replace this line with instructions on how to test your changes, a note
on the devices and browsers this has been tested on, as well as any relevant
images for UI changes.

UI accessibility concerns?

If your PR includes UI changes, please replace this line with details on how
accessibility is impacted and tested. For more info, check out the
Forem Accessibility Docs.

Added/updated tests?

Yes
No, and this is why: please replace this line with details on why tests
have not been included
I need help with writing tests

[Forem core team only] How will this change be communicated?

Will this PR introduce a change that impacts Forem members or creators, the
development process, or any of our internal teams? If so, please note how you
will share this change with the people who need to know about it.

I've updated the Developer Docs or
Storybook (for Crayons components)
This PR changes the Forem platform and our documentation needs to be
updated. I have filled out the
Changes Requested
issue template so Community Success can help update the Admin Docs
appropriately.
I've updated the README or added inline documentation
I've added an entry to
CHANGELOG.md
I will share this change in a Changelog
or in a forem.dev post
I will share this change internally with the appropriate teams
I'm not sure how best to communicate this change and need help
This change does not need to be communicated, and this is why not: please
replace this line with details on why this change doesn't need to be
shared

[optional] Are there any post deployment tasks we need to perform?

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

github-actions · 2021-10-18T03:14:02Z

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!

citizen428 · 2021-10-26T06:34:51Z

app/views/articles/_collection.html.erb

-              <span class="series-switcher__num">...</span>
-              <span class="series-switcher__title"><%= collection_size - 4 %> more <%= "part".pluralize(count: collection_size - 4) %>...</span>
+              title="<%= t("views.articles.series.inbetween.title") %>">
+              <span class="series-switcher__num"><%= t("views.articles.series.inbetween.num") %></span>


I'm not sure this key name is intent-revealing but I also can't come up with a better one 😅

Me neither which is why I took it directly from the style class name above. Any suggestions are welcome.

I guess we can just leave it as is for now.

citizen428 · 2021-10-26T06:37:34Z

app/views/articles/_collection.html.erb

            </a>
        <% end %>

          <a
            href="<%= article.path %>"
            class="crayons-link crayons-link--contentful series-switcher__link <%= "series-switcher__link--active" if rendered_article.id == article.id %> <%= "series-switcher__link--hidden" if collection_size > 5 && (i > 1 && i < collection_size - 2) %>"
            data-preload-image="<%= cloud_cover_url(article.main_image) %>"
-            title="Published <%= article.readable_publish_date %>">
+            title="<%= t("views.articles.series.published", date: article.readable_publish_date) %>">


I think we should probably change the method in Article:

def readable_publish_date relevant_date = displayable_published_at if relevant_date && relevant_date.year == Time.current.year I18n.l(relevant_date, :short) else I18n.l(relevant_date, :short_with_year) end end

If you don't feel comfortable making this change let me know and I make it myself later.

The function as such is independent from views translation used in a lot of places, so I think I'd better defer the implementation details to you.

The function as such is independent from views translation

The function does always return the date as a string though. But anyway, this is something I'll look into later.

citizen428 · 2021-10-27T05:00:15Z

app/views/articles/_collection.html.erb

-              <span class="series-switcher__num">...</span>
-              <span class="series-switcher__title"><%= collection_size - 4 %> more <%= "part".pluralize(count: collection_size - 4) %>...</span>
+              title="<%= t("views.articles.series.inbetween.title") %>">
+              <span class="series-switcher__num"><%= t("views.articles.series.inbetween.num") %></span>


I guess we can just leave it as is for now.

citizen428 · 2021-10-27T05:01:35Z

app/views/articles/_collection.html.erb

            </a>
        <% end %>

          <a
            href="<%= article.path %>"
            class="crayons-link crayons-link--contentful series-switcher__link <%= "series-switcher__link--active" if rendered_article.id == article.id %> <%= "series-switcher__link--hidden" if collection_size > 5 && (i > 1 && i < collection_size - 2) %>"
            data-preload-image="<%= cloud_cover_url(article.main_image) %>"
-            title="Published <%= article.readable_publish_date %>">
+            title="<%= t("views.articles.series.published", date: article.readable_publish_date) %>">


The function as such is independent from views translation

The function does always return the date as a string though. But anyway, this is something I'll look into later.

yheuhtozr · 2021-10-27T12:46:54Z

app/views/articles/_single_story.html.erb

@@ -117,32 +119,32 @@
      <div class="crayons-story__bottom">
        <div class="crayons-story__details">
          <% if story.public_reactions_count > 0 %>
-            <a href="<%= story.path %>" class="crayons-btn crayons-btn--s crayons-btn--ghost crayons-btn--icon-left" data-reaction-count data-reactable-id="<%= story.id %>" aria-label="Comments for post <%= story.title %> (<%= story.public_reactions_count %>)">
+            <a href="<%= story.path %>" class="crayons-btn crayons-btn--s crayons-btn--ghost crayons-btn--icon-left" data-reaction-count data-reactable-id="<%= story.id %>" aria-label="<%= t("views.articles.comments.aria_label", title: story.title, num: story.public_reactions_count) %>">


From #15059 (comment). BTW I find it a bit weird that the three aria labels here and below share the same message, but I just convert them as they are.

maestromac

LGTM despite the tiny thing I pointed out. I'll go ahead and merge it and we can make additional changes in another PR.

maestromac · 2021-10-27T20:13:18Z

app/views/articles/_single_story.html.erb

-          <button type="button" id="article-save-button-<%= story.id %>" class="crayons-btn crayons-btn--secondary crayons-btn--s bookmark-button" data-reactable-id="<%= story.id %>" aria-label="Save to reading list" title="Save to reading list">
-            <span class="bm-initial">Save</span>
-            <span class="bm-success">Saved</span>
+          <button type="button" id="article-save-button-<%= story.id %>" class="crayons-btn crayons-btn--secondary crayons-btn--s w-max bookmark-button" data-reactable-id="<%= story.id %>" aria-label="<%= t("views.articles.save.aria_label") %>" title="<%= t("views.articles.save.title") %>">


was the w-max added by accident?

IIRC that class would save the localized string from unexpected line break. Of course there should be other ways though.

* views/articles (main part) etc i18n * remove ja.yml * delete a replaced core entry * Update en.yml * Update fr.yml * Update _liquid.html.erb * Update _single_story.html.erb * Update en.yml * Update fr.yml * Update _liquid.html.erb * Update discussion_lock_confirm.html.erb * Update _widget_list_item.html.erb * Update _single_story.html.erb

yheuhtozr added 2 commits October 18, 2021 11:59

views/articles (main part) etc i18n

f88da56

remove ja.yml

8bdf72d

pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Oct 18, 2021

yheuhtozr and others added 7 commits October 18, 2021 12:16

delete a replaced core entry

f669ab5

Update en.yml

763ca35

Update fr.yml

d882d90

Update _liquid.html.erb

219d84d

Update _single_story.html.erb

36040d8

Update en.yml

df9c34f

Update fr.yml

3e12a69

citizen428 reviewed Oct 26, 2021

View reviewed changes

yheuhtozr added 3 commits October 26, 2021 18:02

Update _liquid.html.erb

38ef692

Update discussion_lock_confirm.html.erb

b33a5cb

Update _widget_list_item.html.erb

dfc6e04

citizen428 approved these changes Oct 27, 2021

View reviewed changes

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 27, 2021

citizen428 requested a review from djuber October 27, 2021 05:02

yheuhtozr mentioned this pull request Oct 27, 2021

views/comments i18n #15059

Merged

16 tasks

Update _single_story.html.erb

16e64e7

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 27, 2021

yheuhtozr commented Oct 27, 2021

View reviewed changes

yheuhtozr added 2 commits October 27, 2021 21:48

Merge branch 'main' into patch-articles-i18n-14888

7d7058d

Merge branch 'main' into patch-articles-i18n-14888

6372065

maestromac changed the title ~~views/articles (main part) etc i18n~~ I18nize articles & social_previews Oct 27, 2021

maestromac approved these changes Oct 27, 2021

View reviewed changes

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 27, 2021

maestromac merged commit 7613b6c into forem:main Oct 27, 2021

pr-triage bot added PR: merged bot applied label for PR's that are merged and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Oct 27, 2021

djuber mentioned this pull request Oct 27, 2021

Use edited_html to generate article timestamps #15217

Merged

16 tasks

pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: merged bot applied label for PR's that are merged labels Oct 28, 2021

I18nize articles & social_previews #15097

I18nize articles & social_previews #15097

yheuhtozr commented Oct 18, 2021

github-actions bot commented Oct 18, 2021

citizen428 Oct 26, 2021

yheuhtozr Oct 26, 2021

citizen428 Oct 27, 2021

citizen428 Oct 26, 2021

yheuhtozr Oct 26, 2021

citizen428 Oct 27, 2021

citizen428 Oct 27, 2021

citizen428 Oct 27, 2021

yheuhtozr Oct 27, 2021

maestromac left a comment

maestromac Oct 27, 2021

yheuhtozr Oct 28, 2021

I18nize articles & social_previews #15097

I18nize articles & social_previews #15097

Conversation

yheuhtozr commented Oct 18, 2021

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

Description

Related Tickets & Documents

QA Instructions, Screenshots, Recordings

UI accessibility concerns?

Added/updated tests?

[Forem core team only] How will this change be communicated?

[optional] Are there any post deployment tasks we need to perform?

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

github-actions bot commented Oct 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maestromac left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment