Change i18n rendering for edition_count in lazy work preview
#9708
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rebecca-shoptaw
changed the title
edition_count in lazy work previewi18n rendering for edition_count in lazy work preview
6 tasks
cdrini
added
On testing.openlibrary.org
Priority: 1
cdrini
approved these changes
Aug 9, 2024
There was a problem hiding this comment.
Lgtm! https://testing.openlibrary.org/account/lists/add?seeds=OL3497437W,OL3453676W,OL2920993W,OL16016817W,OL17568394W now renders correct 🥳
Souvik-Cyclic
pushed a commit
to Souvik-Cyclic/openlibrary
that referenced
this pull request
Aug 17, 2024
…ernetarchive#9708) * Change variable type for edition count * Revert to if/else and remove variable substitution
SivanC
pushed a commit
to SivanC/openlibrary
that referenced
this pull request
Aug 20, 2024
…ernetarchive#9708) * Change variable type for edition count * Revert to if/else and remove variable substitution
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Subtask of #9486.
Fix. Ensures issue count renders correctly in list edit page.
Technical
As the result of a change in #9506 to improve the
i18ninLazyWorkPreview.html, the list edit page displayed the following error and did not correctly substitute the variable in thei18ntext:I was able to clear the error with the first commit, by switching the
%(count)dto%(count)sso as to correctly type the variable, but the incorrect text (%(count)s editions) was still rendering.I confirmed by trying a variable substitution in the author field that the
jsdefformat does not seem to accept%ssubstitutions ini18ntext, which would explain why for the author field thebyisi18n-ed by itself, instead of following the more traditional$_('by %(author)s', author=author.name)pattern.As a stop-gap measure, I switched the edition field to likewise be
i18n-ed separately, so that the word "edition" or "editions" is displayed conditionally based on the edition count, and is translated separately from the number, so it will always appear after.This should be fine for many languages, but for languages with different word order and/or different numbering systems it's not ideal.
But if there's no other quick fix to improve the
jsdefi18nprocess, this should definitely get the job done for now -- and it's a big advantage over both the error-ed version of the page and the original version in which the word "edition" was not translated at all.Testing
Screenshot
Stakeholders
@cdrini