views/podcasts, views/podcast_episodes i18n #15020

yheuhtozr · 2021-10-12T01:06:50Z

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

Description

Extracts strings for i18n from app/views/podcasts and related. Attached fr locales for testing purposes. Existing translations up to #15002 reflected (hopefully).

Related Tickets & Documents

Relates to #14888

QA Instructions, Screenshots, Recordings

UI accessibility concerns?

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: this is ongoing work related to a large, ongoing initiative.

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

N/A

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

github-actions · 2021-10-12T01:07:05Z

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!

juliannatetreault · 2021-10-14T18:32:59Z

Hey @yheuhtozr! 👋 Can you please update your branch with the latest code from the repository’s main branch? i18n files weren’t being loaded properly and we have merged a fix for this. Thanks so much in advance and please do not hesitate to reach out if you need help!

app/views/podcast_episodes/_podcast_bar.html.erb

citizen428

I think this is looking good, just need to resolve the merge conflicts.

yheuhtozr · 2021-10-19T11:16:46Z

@citizen428 Conflicts "resolved" but GitHub still shows some because of this. Please check the latest change and re-resolve conflicts if you are going to merge this PR.

yheuhtozr · 2021-10-19T11:25:08Z

config/locales/views/podcasts/en.yml

+      unsupported: Your browser does not support the audio element.
+      featured_shows: Featured shows
+      latest_episodes: Latest episodes
+      suggest_a_podcast: Suggest a podcast


These lines are transferred from core.podcasts. Duplication with views.podcasts.form.heading above is intentional, as this is a button text (instruction to user) and the other is a heading (page title).

citizen428 · 2021-10-21T04:04:28Z

@yheuhtozr i18n-tasks missing is not happy with this PR, can you please have a look? This is also why the I18n spec fails (you can run that via bundle exec rspec spec/i18n_spec.rb):

yheuhtozr · 2021-10-21T05:54:44Z

@citizen428 Fixed the keys and the spec files that required now gone keys.

aitchiss

I just have one (hopefully small) suggestion - let me know what you think.

Other than that it looks like there are a couple of small conflicts to resolve and then we should be good to go 🚀

aitchiss · 2021-10-25T10:20:19Z

config/locales/views/podcasts/en.yml

+          one: '1 error while creating a podcast:'
+          other: "%{count} errors while creating a podcast:"
+        owner: I am the owner of this podcast
+        return: "< return to podcasts"


I don't think the < should be part of the actual string here, as we're not using it as "text" as such. Same with the close button "X". These are being used like icons/images (and eventually we will want to replace them with actual icons/images to avoid screen reading software announcing these links as "less than symbol return to podcasts".

Can we extricate them from the translated strings?

Thank you for your input. Do you mean you are going to replace the less than sign with an icon? If so, I'm happy to remove it. But if you only move it out and keep as a part of hardcoded text, I'm not sure it is a good idea. Unlike the close "X", it is not horizontally symmetrical. Some languages are written from right, so that punctuation should be flipped. "<" is not among auto-mirrored symbols and would eventually need to be changed as per locale. At the current stage we only support English and French, but I think the RTL support will be required sooner or later.

That's a really good point. I think in light of that, it's best to carry on with the implementation you have here and I'll speak to our team about how we want to adjust things in future, as eventually we don't want text representations of iconography (as they're not properly providing information to screen reader users). Until we make that future change, best to keep things together in the translated strings, as you say.

(if you're able to resolve those couple of conflicts, I'll approve and get this merged 👍 )

* views/podcasts, views/podcast_episodes i18n * reformat for PR * helper labels * PR sync with main * remove ja.yml * Update en.yml * Update fr.yml * Update _meta.html.erb * Update podcast_episodes_index_spec.rb * Update podcast_create_spec.rb * Update user_visits_podcast_episode_spec.rb * Update en.yml * Update feed.rb * Update index.html.erb

views/podcasts, views/podcast_episodes i18n

9dda6af

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

benhalpern mentioned this pull request Oct 12, 2021

Fix and clean up podcast pages #15004

Merged

16 tasks

citizen428 reviewed Oct 15, 2021

View reviewed changes

app/views/podcast_episodes/_podcast_bar.html.erb Outdated Show resolved Hide resolved

reformat for PR

aa9a32e

yheuhtozr force-pushed the patch-podcasts-i18n-14888 branch from 50de47b to d5c1324 Compare October 15, 2021 10:59

juliannatetreault requested a review from benhalpern October 15, 2021 16:09

helper labels

aaddc48

yheuhtozr force-pushed the patch-podcasts-i18n-14888 branch from d5c1324 to d512c96 Compare October 18, 2021 06:03

citizen428 approved these changes Oct 19, 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 19, 2021

yheuhtozr force-pushed the patch-podcasts-i18n-14888 branch from d512c96 to 896818f Compare October 19, 2021 10:56

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

yheuhtozr added 2 commits October 19, 2021 20:19

PR sync with main

1230029

remove ja.yml

86158e6

yheuhtozr force-pushed the patch-podcasts-i18n-14888 branch from 896818f to 86158e6 Compare October 19, 2021 11:20

yheuhtozr commented Oct 19, 2021

View reviewed changes

citizen428 requested a review from rhymes October 20, 2021 08:53

yheuhtozr force-pushed the patch-podcasts-i18n-14888 branch from bf07bfa to 86158e6 Compare October 20, 2021 08:57

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

fa27012

yheuhtozr added 4 commits October 21, 2021 13:30

Update en.yml

7bb2b84

Update fr.yml

48d7383

Update _meta.html.erb

0055acf

Update podcast_episodes_index_spec.rb

0f4130c

yheuhtozr added 2 commits October 21, 2021 14:49

Update podcast_create_spec.rb

1a754f2

Update user_visits_podcast_episode_spec.rb

336b1e2

yheuhtozr added 2 commits October 22, 2021 01:10

Update en.yml

ba29a7f

Update feed.rb

d00dcc3

citizen428 requested review from aitchiss and removed request for rhymes October 25, 2021 08:27

aitchiss reviewed Oct 25, 2021

View reviewed changes

yheuhtozr added 2 commits October 25, 2021 22:07

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

d608949

Update index.html.erb

5353e10

aitchiss approved these changes Oct 25, 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 25, 2021

aitchiss merged commit 6a3a91d into forem:main Oct 25, 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 25, 2021

views/podcasts, views/podcast_episodes i18n #15020

views/podcasts, views/podcast_episodes i18n #15020

yheuhtozr commented Oct 12, 2021 •

edited by juliannatetreault

github-actions bot commented Oct 12, 2021

juliannatetreault commented Oct 14, 2021 •

edited

citizen428 left a comment •

edited

yheuhtozr commented Oct 19, 2021

yheuhtozr Oct 19, 2021 •

edited

citizen428 commented Oct 21, 2021 •

edited

yheuhtozr commented Oct 21, 2021

aitchiss left a comment

aitchiss Oct 25, 2021

yheuhtozr Oct 25, 2021

aitchiss Oct 25, 2021

aitchiss Oct 25, 2021

views/podcasts, views/podcast_episodes i18n #15020

views/podcasts, views/podcast_episodes i18n #15020

Conversation

yheuhtozr commented Oct 12, 2021 • edited by juliannatetreault

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

juliannatetreault commented Oct 14, 2021 • edited

citizen428 left a comment • edited

Choose a reason for hiding this comment

yheuhtozr commented Oct 19, 2021

yheuhtozr Oct 19, 2021 • edited

Choose a reason for hiding this comment

citizen428 commented Oct 21, 2021 • edited

yheuhtozr commented Oct 21, 2021

aitchiss left a comment

Choose a reason for hiding this comment

aitchiss Oct 25, 2021

Choose a reason for hiding this comment

yheuhtozr Oct 25, 2021

Choose a reason for hiding this comment

aitchiss Oct 25, 2021

Choose a reason for hiding this comment

aitchiss Oct 25, 2021

Choose a reason for hiding this comment

yheuhtozr commented Oct 12, 2021 •

edited by juliannatetreault

juliannatetreault commented Oct 14, 2021 •

edited

citizen428 left a comment •

edited

yheuhtozr Oct 19, 2021 •

edited

citizen428 commented Oct 21, 2021 •

edited