AO3-7548 i18n tagset-about, tagset-tag-associations and tagset-batch-load help files by sherin · Pull Request #5981 · otwcode/otwarchive

sherin · 2026-08-13T18:38:09Z

…_tag_sets/index

Pull Request Checklist

Have you read the contributing guidelines?
Have you read "How to write the perfect pull request"?
Have you added tests for any changed functionality?
Have you added the Jira issue number
as the first thing in your pull request title (e.g. AO3-1234 Fix thing)
Do you have fewer than 5 pull requests already open? If not, please wait
until they are reviewed and merged before creating new pull requests.
I acknowledge that submitting code created or modified with the help of AI is a bannable offense.

Issue

https://otwarchive.atlassian.net/browse/AO3-7548

Purpose

Move 3 help files over to erb with i18n compliance. In addition, for owned_tag_sets/index.html, I've also fixed and moved its translation strings to the en locale file (converting ts calls to t).

Testing Instructions

Refer ticket

Credit

sherin

…_tag_sets/index

ASalbenblatt

Just a few small things, but overall looking really good!

I also transitioned your Jira issue to In Review and assigned you to it so no one else picks it up.

ASalbenblatt · 2026-08-24T02:59:11Z

            acronym: non-con
            title: non-consensual sex
+    tag_sets_about:
+      delete_tag_set_warning_strong: can delete the tag set or change it without warning


The _strong suffix might be confusing to a translator who doesn't know HTML. The way you're supposed to do this is to make the key here just delete_tag_set_warning and then have the variable be something descriptive so they see %{delete_tag_set_warning_bold} in the other string.

good point, thank you

ASalbenblatt · 2026-08-24T03:11:06Z

-    
+
  <% end %>
 </div>


Since you're already modifying this file pretty heavily could you please add the missing newline at the end here

ASalbenblatt · 2026-08-24T03:30:56Z


  <fieldset class="tagset">
-    <legend>Tags In Set <% unless @tag_set.new_record? || !@tag_set.tag_set || @tag_set.tag_set.tags.empty? %><%= ts('(check to remove)') %><% end %></legend>
+    <legend>Tags In Set <% unless @tag_set.new_record? || !@tag_set.tag_set || @tag_set.tag_set.tags.empty? %><%= t(".check_to_remove") %><% end %></legend>


Our i18n standards say that even if a text fragment is identical in two places it should be put in two different locales like .check_to_remove_tags and .check_to_remove_associations in case they end up being different in another language (like maybe tags and associations have different genders and that requires remove to be conjugated differently or something)

that makes sense, thank you!

ASalbenblatt · 2026-08-24T03:38:08Z

+        <dt class="landmark"><%= t(".submit") %></dt>
+        <dd class="submit actions"><%= form.submit t(".submit") %></dd>


These I think are fine to be duplicated because they are both referring to the same button

.submit.landmark and .submit.button would be better, since they're different elements and different contexts. At some point, we'll likely change the button to less generic text like "Batch Load," but we'd still want the landmark to indicate that it is the submit button.

ASalbenblatt · 2026-08-24T03:58:03Z

+      <li><%= link_to t(".new_tag_set_link"), new_tag_set_path %></li>
+      <li><%= link_to t(".tags_link"), tags_path %></li>


These are probably better described as _button instead of _link

ASalbenblatt

Looks great, thanks!

sarken

Thanks for working on this! Just a few notes on some of our standards.

sarken · 2026-09-03T05:27:09Z

    <% if logged_in? %>
-  	  <li><%= link_to ts('New Tag Set'), new_tag_set_path %></li>
-  	  <li><%= link_to ts('Tags'), tags_path %></li>
+      <li><%= link_to t(".new_tag_set_button"), new_tag_set_path %></li>


Just a heads up that these are links, so they shouldn't be called button -- there might come a time where we don't style them as buttons, which would make this name confusing.

However, we actually have an established naming pattern for the subnavigation links that avoids the question entirely: .navigation.link_text (or a shortened form of the link text). So here the best keys would probably be:

.navigation.search.submit (and .navigation.search.title for the title attribute)

.navigation.new_tag_set

.navigation.tags

sarken · 2026-09-03T05:28:22Z

  <ul class="navigation actions" role="navigation">
    <li class="search" role="search">
-      <%= form_tag tag_sets_path, :class => "simple search", :method => :get do |form| %>
+      <%= form_tag tag_sets_path, class: "simple search", method: :get do |form| %>


Can you remove the role attributes from the ul and li as long as you're here? They're actually invalid.

sarken · 2026-09-03T05:31:35Z

  <h2 class="heading">
    <% if @user %>
-      <%= ts('%{user_name}\'s Tag Sets', :user_name => @user.login) %> <%= link_to_help 'tagset-about' %>
+      <%= t(".page_heading_user", user_name: @user.login) %> <%= link_to_help_modal(help_tag_sets_about_path, t(".tag_sets_about_help_title")) %>


When there's more than one possible page heading, we generally prefer to do .page_heading.user, page_heading.query, etc.

sarken · 2026-09-03T05:33:07Z

+    batch_load:
+      page_heading: Batch Loading
+      submit: Submit
+      tag_sets_batch_load_help_title: Tagset batch load


Can you make "tagset" two words here, and in tag_sets_about_help_title and tag_sets_associations_help_title? We never actually want to say "tagset;" it was just an undesirable consequence of relying on the file name.

sarken · 2026-09-03T05:34:09Z

+      page_heading_query: Tag Set
+      page_heading_user: "%{user_name}'s Tag Sets"
+      search_submit: Search
+      search_title: search


We're moving away from the hardcoded lowercase text that's leftover from the 2010s, so this can be capitalized.

sarken · 2026-09-03T05:49:39Z

+      <%= search_header @tag_sets, nil, t(".page_heading_query") %> <%= link_to_help_modal(help_tag_sets_about_path, t(".tag_sets_about_help_title")) %>
    <% else %>
-      <%= ts('Tag Sets') %> <%= link_to_help 'tagset-about' %> <%= ts('in the %{archive_name}', :archive_name => ArchiveConfig.APP_NAME) %>
+      <%= t(".page_heading_default") %> <%= link_to_help_modal(help_tag_sets_about_path, t(".tag_sets_about_help_title")) %> <%= t(".in_archive_name", archive_name: ArchiveConfig.APP_NAME) %>


Splitting up a phrase or sentence can make translation difficult, so this should be one string with the modal link as a variable. (You can refer to the Variable interpolation section of our i18n standards page for some examples.)

sarken · 2026-09-03T05:54:55Z

+        <dt class="landmark"><%= t(".submit") %></dt>
+        <dd class="submit actions"><%= form.submit t(".submit") %></dd>


.submit.landmark and .submit.button would be better, since they're different elements and different contexts. At some point, we'll likely change the button to less generic text like "Batch Load," but we'd still want the landmark to indicate that it is the submit button.

sarken · 2026-09-03T05:59:28Z

    <% end %>

-    <h3 class="landmark heading"><%= ts('Listing Tag Sets') %></h3>
+    <h3 class="landmark heading"><%= t(".listing_tag_sets_heading") %></h3>


We usually call these .landmark or .landmark.something (since there's often more than one landmark on a page).

move tag set about help to be i18n compatible. move ts -> t for owned…

003f0cd

…_tag_sets/index

github-actions Bot added the Awaiting Review label Aug 13, 2026

sherin changed the title ~~move tag set about help to be i18n compatible. move ts -> t for owned…~~ AO3-7548 move tag set about help to be i18n compatible. move ts -> t for owned… Aug 13, 2026

sherin added 9 commits August 13, 2026 11:44

erb lint fixes

54e3442

erb lint fix

73675ad

add i18n support for tag sets association help

46ee1f2

erb lint fix

daeec30

i18n support for batch load in help

8ffcf47

erb lint fixes

9966318

another lint fix - linter is being funky

3a8be21

lint error

e00b198

argh fix

8071150

sherin marked this pull request as ready for review August 15, 2026 05:21

sarken changed the title ~~AO3-7548 move tag set about help to be i18n compatible. move ts -> t for owned…~~ AO3-7548 i18n tagset-about, tagset-tag-associations and tagset-batch-load help files Aug 19, 2026

ASalbenblatt suggested changes Aug 24, 2026

View reviewed changes

address feedback

67fa10f

ASalbenblatt approved these changes Aug 24, 2026

View reviewed changes

sarken requested changes Sep 3, 2026

View reviewed changes

sarken added Reviewed: Action Needed and removed Awaiting Review labels Sep 3, 2026

		<dt class="landmark"><%= t(".submit") %></dt>
		<dd class="submit actions"><%= form.submit t(".submit") %></dd>

		<li><%= link_to t(".new_tag_set_link"), new_tag_set_path %></li>
		<li><%= link_to t(".tags_link"), tags_path %></li>

Uh oh!

Conversation

sherin commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist

Issue

Purpose

Testing Instructions

Credit

Uh oh!

ASalbenblatt left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ASalbenblatt left a comment

Choose a reason for hiding this comment

Uh oh!

sarken left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sherin commented Aug 13, 2026 •

edited

Loading

ASalbenblatt left a comment •

edited

Loading