Steps to reproduce

  • Install the site:
    ddev poser
ddev symlink-project
ddev install
  • Install config translation: ddev drush en config_translation
  • Go to https://babel.ddev.site/admin/config/regional/babel
  • Fill "Search" with "Anonymous" and check "Show inactive"
  • Press "Filter". You'll get the records containing "anonymous" string (query string is ?langcode=it&translated=all&search=Anonymous&include_inactive=1)
  • DON'T reload the page, keep the browser tab open!!!
  • In a different tab go to https://babel.ddev.site/admin/config/people/accounts/translate
  • Add the Italian translation of the user.settings:anonymous property.
  • Translate "Anonymous" as "Anonymous IT". Press "Save translation"
  • Switch tho the 1st tab but DON'T reload the page!
  • Press "Filter".

I'm expecting to see the translation of "Anonymous" as "Anonymous IT" but the field is still empty. Being an Ajax submit, which makes a server roundtrip, I'm expecting the translation field to be updated

BUT, if you reload the page, the translation appears.

This happens also with Locale strings, so it's not related to the used plugin... For instance, search for "Image styles", go to /admin/config/regional/translate and translate the same string, then press "Filter". Will notice the same behavior, and, again, on full page reload it works.

Issue fork babel-3549355

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

claudiu.cristea created an issue. See original summary.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Issue summary: View changes

dxvargas made their first commit to this issue’s fork.

dxvargas opened merge request !61

dxvargas’s picture

dxvargas commented
Status: Active » Needs review

I've made a small change that fixed the issue.
Ready for review.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented

@claudiu.cristea As the author of the current form (behavior), my intention was to keep user input between AJAX rounds.

Scenario:

  1. User visits translation form
  2. Changes the translation of "String A"
  3. Changes the translation of "String B"
  4. Clicks the save at "String A"

Without the change in this MR, the input at "String B" was kept.
The linked MR changes this behavior, so every time you save / delete a translations, all translation inputs are reloaded with the actual data from DB.

You have to decide if you're fine with this change.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Assigned: Unassigned » claudiu.cristea
claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Assigned: claudiu.cristea » huzooka

I'm fine because we made now everything faster in #3548484: Improve ::getStrings() performance EDIT: Sorry, I've misread the above remark. Indeed that's a problem, I didn't test this issue.

I think we need both, to keep the input on "Filter" and also to bring updates from outside. In case of conflict the input should win (not 100% sure, but it seems the correct resolution). And we need to test this.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Status: Needs review » Needs work

@dxvargas Can you please check whether it's possible?

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Assigned: huzooka » Unassigned
dxvargas’s picture

dxvargas commented

Answering to @claudiu.cristea in #8.

I think we need both, to keep the input on "Filter" and also to bring updates from outside. In case of conflict the input should win (not 100% sure, but it seems the correct resolution).

If we have a translation in the form input that is different from the storage, how can we decide which one is correct?

  1. Maybe the user has changed the translation in the input, then the input value should be kept.
  2. Maybe the translation was changed outside the form, then the input should be updated (that's the goal of this issue).

To make the distinction between these two possibilities, we would need to keep an hidden input with the original translation.
If this original translation is different from the received translation and different from storage, we know we are in case 1.
If this original translation is the same as the received translation and different from storage, we know we are in case 2.

But I think this is too complex.

I propose an other behavior that I expect that is easier to implement and to understand:

  • If the user saves a record, we don't automatically update any input in the form.
  • If the user filters the list, every record is automatically updated with the values from storage. In that case, any unsaved translation is lost.

I think this simplifies the process and it's an expected behavior. It complies with the initial plan from @huzooka, explained in #6, and also with the description in the current issue.

dxvargas’s picture

dxvargas commented
Assigned: Unassigned » dxvargas

I'll give it a try with the goal explained by @claudiu.cristea in #8.

dxvargas’s picture

dxvargas commented
Assigned: dxvargas » Unassigned
Status: Needs work » Needs review

I've addressed the remarks in #8:

  1. I've adapted the code to keep the changes done by the user.
  2. I've added tests.

Looking forward for reviews!

dxvargas’s picture

dxvargas commented
Assigned: Unassigned » dxvargas
Status: Needs review » Needs work

I need to cover some edge cases. For example, when the original translation was empty.

dxvargas’s picture

dxvargas commented
Assigned: dxvargas » Unassigned
Status: Needs work » Needs review

I've tested as much as I could. Added another test. Review please.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

Brilliant, thank you @dxvargas for all your efforts!

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Assigned: Unassigned » huzooka
Status: Fixed » Reviewed & tested by the community

Reopening and settings RTBC because the merged HEAD revealed test failures. I will check what's going on locally.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented

To me it seems we have a random error.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Assigned: huzooka » Unassigned
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.