Implement pluralized add new translation string on web UI #11756

harriebird · 2024-05-30T22:03:53Z

Proposed changes

This changes the way how the adding of new translation string done in the web UI.
This adds an option to add pluralized translation string.
This solves issue #7663.

Example image:

Checklist

Lint and unit tests pass locally with my changes.
I have added tests that prove my fix is effective or that my feature works.
I have added documentation to describe my feature.
I have squashed my commits into logic units.
I have described the changes in the commit messages.

Other information

codecov · 2024-05-31T00:01:45Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.62%. Comparing base (6aee586) to head (8b114d8).
Report is 2251 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11756      +/-   ##
==========================================
- Coverage   90.82%   90.62%   -0.20%     
==========================================
  Files         554      577      +23     
  Lines       57306    58856    +1550     
  Branches     9122     9395     +273     
==========================================
+ Hits        52046    53340    +1294     
- Misses       3640     3824     +184     
- Partials     1620     1692      +72

Files	Coverage Δ
weblate/trans/forms.py	`88.61% <100.00%> (+0.75%)`	⬆️
weblate/trans/models/translation.py	`86.74% <100.00%> (-0.47%)`	⬇️
weblate/trans/tests/test_edit.py	`100.00% <100.00%> (ø)`
weblate/trans/views/basic.py	`82.73% <ø> (-0.20%)`	⬇️

... and 330 files with indirect coverage changes

nijel

Looks good, I've made some minor comments.

nijel · 2024-06-13T18:33:54Z

weblate/lang/models.py

@@ -663,6 +663,10 @@ def is_base(self, vals: tuple[str, ...]) -> bool:
        """Detect whether language is in given list, ignores variants."""
        return self.base_code in vals

+    def get_source_plurals(self):
+        """Return blank source fields for pluralized new string."""
+        return ["" for _ in range(self.plural.number)]


Suggested change

return ["" for _ in range(self.plural.number)]

return [""] * self.plural.number

Applied the recommended change in the code.

nijel · 2024-06-13T18:36:00Z

weblate/templates/translation.html

+    </a>
+    <ul class="dropdown-menu">
+      <li><a href="#new-singular" data-toggle="tab">{% trans "Singular" %}</a></li>
+      <li><a href="#new-plural" data-toggle="tab">{% trans "Plural" %}</a></li>


I think the plural choice should be available only for languages having plural (plural.number > 1). Otherwise, there would be two same forms.

I agree, updated the code the new plural translation string. The form for new plural string and those buttons that leads it will be only shown if plural.number > 1.

nijel · 2024-06-17T12:39:57Z

weblate/templates/translation.html

@@ -74,9 +74,6 @@
      {% endif %}
      <li><a href="{% url 'data_project' project=object.component.project.slug  %}">{% trans "Data exports" %}</a></li>
      <li><a href="{% url 'checks' path=object.get_url_path %}">{% trans "Failing checks" %}</a></li>
-      {% if user_can_add_unit %}
-      <li><a href="#new" data-toggle="tab">{{ object.component.get_add_label }}</a></li>
-      {% endif %}


I'd really prefer to keep it this way so that link to #new still works.

Reverted to the previous #new hyperlink to a tab containing the new Add new translation string form.

nijel · 2024-06-17T12:40:39Z

weblate/templates/translation.html

+      {% endif %}
+    </ul>
+    </li>
+  {% endif %}


The top-level navigation has already enough items, so better to stick with placement in the Tools menu.

Reverted this change.

nijel · 2024-06-17T12:41:30Z

weblate/templates/translation.html

+        </div>
+      </form>
+    </div>
+  {% endif %}


Can we have it on a single tab and toggle the forms there (maybe radio)?

Yes, It is now only a single tab. The form can be switched by selecting Singular or Plural radio buttons.

nijel · 2024-06-17T12:43:31Z

weblate/lang/models.py

@@ -663,6 +663,10 @@ def is_base(self, vals: tuple[str, ...]) -> bool:
        """Detect whether language is in given list, ignores variants."""
        return self.base_code in vals

+    def get_source_plurals(self):
+        """Return blank source fields for pluralized new string."""
+        return [""] * self.plural.number


The default language plural doesn't have to be the one which is actually being used, this should belong to the Translation class to get correct Plural instance.

Transferred the function get_source_plurals to Translation model.

nijel · 2024-06-17T12:44:32Z

weblate/trans/forms.py

-def get_new_unit_form(translation, user, data=None, initial=None):
+def get_new_unit_form(
+    translation, user, data=None, initial=None, is_source_plural=None
+):


I guess that is_source_plural should be passed in all branches here, not just the one. Currently, the Singular and Plural forms are the same for me on monolingual translation.

Added is_source_plural to NewMonolingualUnitForm to address this.

nijel · 2024-06-19T07:39:31Z

weblate/trans/forms.py

+            data=data,
+            initial=initial,
+            is_source_plural=is_source_plural,
+        )
    return NewBilingualUnitForm(translation, user, data=data, initial=initial)


At least this also needs the logic (when adding string to bilingual translation), it still lacks the plural fields:

I'm not sure about the glossary forms above. Technically, there is no reason not to use plurals, but the default glossary format doesn't support it.

Figured this out, both source and target in the NewBilingualUnitForm must receive the value of is_source_plural.

nijel · 2024-06-19T07:42:04Z

weblate/static/loader-bootstrap.js

+      $("#new-singular").addClass("hidden");
+      $("#new-plural").removeClass("hidden");
+    }
+  });


Can this also sync forms content? So that the text doesn't disappear when you toggle?

Yes, this can now transfer the inputs when toggling between the singular and plural forms.

nijel · 2024-06-20T07:16:52Z

Tests need some adjustment:

======================================================================
FAIL: test_bilingual_new_plural_unit (weblate.trans.tests.test_edit.EditDTDTest.test_bilingual_new_plural_unit)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/weblate/weblate/weblate/trans/tests/test_edit.py", line 242, in test_bilingual_new_plural_unit
    self.test_new_plural_unit(args, "cs")
  File "/home/runner/work/weblate/weblate/weblate/trans/tests/test_edit.py", line 220, in test_new_plural_unit
    self.assertContains(
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/django/test/testcases.py", line 524, in assertContains
    text_repr, real_count, msg_prefix = self._assert_contains(
                                        ^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/django/test/testcases.py", line 487, in _assert_contains
    self.assertEqual(
AssertionError: 403 != 200 : Couldn't retrieve content: Response code was 403 (expected 200)

Most likely, the failures are for formats not supporting plurals so the tests should assert that plurals do not work in that case?

See https://github.com/WeblateOrg/weblate/actions/runs/9583865006/job/26426111254?pr=11756

for more information, see https://pre-commit.ci

….get_source_plurals

for more information, see https://pre-commit.ci

…rce_plurals

…radio buttons

…sponding test

harriebird · 2024-06-20T09:43:44Z

Hi @nijel, I can't directly reply on your comment about the test failure. I adjusted the test to skip those formats that are not supported.

nijel · 2024-06-25T08:23:04Z

Merged, thanks for your contribution!

nijel · 2024-07-12T12:39:41Z

weblate/templates/translation.html

+          <label class="radio-inline">
+            <input type="radio" name="new-unit-form-type" id="show-singular" value="singular" checked> {% trans "Singular" %}
+          </label>
+          <label class="radio-inline">
+            <input type="radio" name="new-unit-form-type" id="show-plural" value="plural"
+              {% if object.plural.number < 2 %} disabled{% endif %}> {% trans "Plural" %}
+          </label>


These radios should be hidden if there is just a single form available (show only if {% if object.plural.number > 1 %}).

harriebird force-pushed the feature/allow-pluralized-translation-added-from-webapp branch 2 times, most recently from 2ae47ab to 9cc3dd4 Compare May 31, 2024 17:42

harriebird marked this pull request as ready for review May 31, 2024 18:46

harriebird mentioned this pull request May 31, 2024

Is there is a way to create pluralized translations from the UI? #7663

Closed

2 tasks

harriebird force-pushed the feature/allow-pluralized-translation-added-from-webapp branch from 529dcc8 to f4fd982 Compare May 31, 2024 19:01

harriebird requested a review from orangesunny as a code owner May 31, 2024 19:26

harriebird force-pushed the feature/allow-pluralized-translation-added-from-webapp branch 2 times, most recently from a1a44b0 to e09a29b Compare June 2, 2024 16:05

nijel reviewed Jun 13, 2024

View reviewed changes

nijel added this to the 5.6 milestone Jun 13, 2024

nijel linked an issue Jun 13, 2024 that may be closed by this pull request

Is there is a way to create pluralized translations from the UI? #7663

Closed

2 tasks

harriebird force-pushed the feature/allow-pluralized-translation-added-from-webapp branch from cc0c79d to 30577f8 Compare June 14, 2024 12:50

nijel modified the milestones: 5.6, 5.7 Jun 17, 2024

nijel reviewed Jun 17, 2024

View reviewed changes

harriebird force-pushed the feature/allow-pluralized-translation-added-from-webapp branch from 30577f8 to 2c0e098 Compare June 18, 2024 14:58

nijel reviewed Jun 19, 2024

View reviewed changes

harriebird and others added 11 commits June 20, 2024 16:27

Implemented add plural new string on web UI

bf9c5ec

[pre-commit.ci] auto fixes from pre-commit.com hooks

96ed07f

for more information, see https://pre-commit.ci

Added get_source_plurals in Language model

c74ea4e

[pre-commit.ci] auto fixes from pre-commit.com hooks

a5e7437

for more information, see https://pre-commit.ci

Added test

8a4d058

[pre-commit.ci] auto fixes from pre-commit.com hooks

b8e113a

for more information, see https://pre-commit.ci

Removed unnecessary print

aebe1fc

Updated get_source_plurals docstring in Language model

50cad26

Removed unnecessary assignment before the return statment in Language…

674e79d

….get_source_plurals

Updated the docs regarding this new changes

ae010d1

Improved the test

f541d1d

pre-commit-ci bot and others added 9 commits June 20, 2024 16:27

[pre-commit.ci] auto fixes from pre-commit.com hooks

e7c9a8b

for more information, see https://pre-commit.ci

Updated new plural translation string visibility and improved get_sou…

ae63056

…rce_plurals

Restored the new unit under Tools and made the form switchable using …

3754f1a

…radio buttons

Used the plural property in Translation object to get the plural.number

5324881

Added is_source_plural for monolingual new source unit

3b7cd77

Updated the docs relevant to the changes made

fa0b9a3

Added support for plural source in NewBilingualUnitForm and its corre…

e9691cb

…sponding test

Enabled input values sync when toggling between plural and singular form

4e82180

Added condition on test_bilingual_new_plural_unit to check for bilingual

8b114d8

harriebird force-pushed the feature/allow-pluralized-translation-added-from-webapp branch from 1b8c139 to 8b114d8 Compare June 20, 2024 08:27

nijel merged commit dd540b1 into WeblateOrg:main Jun 25, 2024
29 of 31 checks passed

nijel self-assigned this Jun 25, 2024

timothyqiu mentioned this pull request Jul 11, 2024

Plural fields missing when adding glossary #12044

Closed

2 tasks

nijel reviewed Jul 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement pluralized add new translation string on web UI #11756

Implement pluralized add new translation string on web UI #11756

harriebird commented May 30, 2024 •

edited

Loading

codecov bot commented May 31, 2024 •

edited

Loading

nijel left a comment

nijel Jun 13, 2024

harriebird Jun 14, 2024

nijel Jun 13, 2024

harriebird Jun 14, 2024

nijel Jun 17, 2024

harriebird Jun 18, 2024

nijel Jun 17, 2024

harriebird Jun 18, 2024

nijel Jun 17, 2024

harriebird Jun 18, 2024

nijel Jun 17, 2024

harriebird Jun 20, 2024

nijel Jun 17, 2024

harriebird Jun 18, 2024

nijel Jun 19, 2024

harriebird Jun 20, 2024

nijel Jun 19, 2024

harriebird Jun 20, 2024

nijel commented Jun 20, 2024

harriebird commented Jun 20, 2024

nijel commented Jun 25, 2024

nijel Jul 12, 2024 •

edited

Loading

	return ["" for _ in range(self.plural.number)]
	return [""] * self.plural.number

Implement pluralized add new translation string on web UI #11756

Implement pluralized add new translation string on web UI #11756

Conversation

harriebird commented May 30, 2024 • edited Loading

Proposed changes

Checklist

Other information

codecov bot commented May 31, 2024 • edited Loading

Codecov Report

nijel left a 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nijel commented Jun 20, 2024

harriebird commented Jun 20, 2024

nijel commented Jun 25, 2024

nijel Jul 12, 2024 • edited Loading

Choose a reason for hiding this comment

harriebird commented May 30, 2024 •

edited

Loading

codecov bot commented May 31, 2024 •

edited

Loading

nijel Jul 12, 2024 •

edited

Loading