- Issue created by @claudiu.cristea
- π·π΄Romania claudiu.cristea Arad π·π΄
When this is in "Needs review" stage, the reviewer will perform profiling and, if case, will open a follow-up to streamline the search by keyword
- ππΊHungary huzooka Hungary ππΊπͺπΊ
API changes:
- \Drupal\babel\Model\StringTranslation::$translation is not read-only anymore: Since translations can be changed and removed, it makes no sense do force ourselves to destroy the original and reinstantiate a replacement object on a translation change / deletion. This simplified the translation import form a lot, because we don't have to update the corresponding StringTranslation object at the array key in the form state storage.
- \Drupal\babel\Plugin\Babel\TranslationTypePluginInterface::updateTranslation does not return anything anymore. Before my MR, it returned a StringTranslation object when a translation was added or modified, or just returned NULL if translation was deleted.
Functional changes:
- Form wasn't functional without JavaScript - this is now fixed.
- To make the translate form's new pager work without page refresh, I added a dedicated render element, a dedicated theme function and a dedicated route. In the meantime, I had to add workarounds for π Fixed pagination and sorting of tables on pages with ajax Needs work .
- I'm still not satisfied with the filtering; I am thinking about creating separate form for it.
- ππΊHungary huzooka Hungary ππΊπͺπΊ
Filtering now works with AJAX-submit even if user hits enter;
Found a way to disable filter element refocus (this was very annoying when I was using pointer device).I added both a non-JS and a JS test for the UI.
Asking for a first round of QA.
- π΅π±Poland alorenc Wolsztyn, π΅π±
After enabling two languages on a fresh instance, I visited /admin/config/regional/babel and encountered the following fatal error:
The website encountered an unexpected error. Try again later.
AssertionError: assert(count($form['list']['#header']) === count($form['list'][$keys])) in assert() (line 252 of modules/repos/babel/src/Form/BabelTranslateForm.php).
Drupal\babel\Form\BabelTranslateForm->buildForm()
call_user_func_array() (Line: 528) - π΅π±Poland alorenc Wolsztyn, π΅π±
I have found another error.
Symfony\Component\HttpKernel\Exception\HttpException: The specified #ajax callback is empty or not callable. in Drupal\Core\Form\FormAjaxResponseBuilder->buildResponse() (line 67 of /var/www/html/web/core/lib/Drupal/Core/Form/FormAjaxResponseBuilder.php).
Steps:
* visit /admin/config/regional/babel
* type something in search box and press enter
* system logs a fatal error - ππΊHungary huzooka Hungary ππΊπͺπΊ
Forgot to set the right assignee.
- ππΊHungary huzooka Hungary ππΊπͺπΊ
Asking a second review from Adrian.
I propose to solve https://www.drupal.org/project/babel/issues/3533685#mr17-note562236 π Refactor translation UI/form Active (history pushstate AJAX command?) and https://www.drupal.org/project/babel/issues/3533685#mr17-note562245 π Refactor translation UI/form Active (pager reset on filter change) in dedicated tickets.
Also, if this ticket is accepted, I have to create 2 more follow-up tickets with some code removal if core bugs are resolved ( π Fixed pagination and sorting of tables on pages with ajax Needs work and π Ajax callbacks on paginated forms with PagerSelectExtender not updating on first callback Active )
- π΅π±Poland alorenc Wolsztyn, π΅π±
Fatal errors and my remarks were resolved.
Found another issue:
* /admin/config/regional/babel
* click on "Translate" from the top toolbar
* modal is displayed on top of /admin/config/regional/babel
* I clicked around and was redirected to babel/pager/ajax page
Do we want to create another follow up? - π΅π±Poland alorenc Wolsztyn, π΅π±
When I update the string at /admin/config/regional/babel, I can see a message.
However, when I use the modal (by clicking Translate from the toolbar), the messages are not displayed. Is this expected? - π΅π±Poland alorenc Wolsztyn, π΅π±
The search string is lost during the following steps:
* Click Translate from the toolbar.
* Paste, for example, "Behavior settings" into the search field.
* Press the Filter button.
* The content updates, but the entered search string is lost. - ππΊHungary huzooka Hungary ππΊπͺπΊ
Unfortunately we also hit π Error: Cannot read properties of undefined (reading 'settings')β¨ with dialog.position.js Active after the first AJAX operation performed within the modal dialog, so I also add the core issue as related.
- ππΊHungary huzooka Hungary ππΊπͺπΊ
Changes:
- Addressed #19 in a way that seems obvious to me: not showing toolbar link on the translate form's path. Also I found some inconsistencies with the toolbar link β it was shown for users without the translate permission. Fixed them in this ticket.
- Fixed #20. (BTW the message was shown, but unfortunately not in the modal; it was added to the page.)
- Also added another workaround for #21: If an AJAX operation is already in progress, we don't let another to be started additionally. Ideally, an AJAX operation targeting a form element should disable all other interactive element but unfortunately it only disables the initiator element.
- Added another test to cover the toolbar link and to test the translate form's basic functionality within the modal
Proposed follow-up tickets to be created (note for mostly myself):
- History pushState AJAX command (see https://www.drupal.org/project/babel/issues/3533685#mr17-note562236 π Refactor translation UI/form Active )
- Reset pager when filter is changed (see https://www.drupal.org/project/babel/issues/3533685#mr17-note562245 π Refactor translation UI/form Active )
- Remove workarounds of π Fixed pagination and sorting of tables on pages with ajax Needs work when core issue is resolved
- Remove workarounds of π Ajax callbacks on paginated forms with PagerSelectExtender not updating on first callback Active when core issue is resolved
- Remove workarounds of π Find a generic way to resolve race condition on AJAX change event and form submission Active
- π΅π±Poland alorenc Wolsztyn, π΅π±
Pagination sometimes disappears after specific user actions.
Steps to reproduce:
* Click on any page.
* Click on "Show inactive".
* Press Save for a selected translation
* Pagination disappears, but I expect the user to remain on the same page, with only the selected row being updated. - ππΊHungary huzooka Hungary ππΊπͺπΊ
The filter loss mentioned in #27 was caused by a race condition between two AJAX operation what I wasn't able to prevent. First operation was started by hitting ENTER; the second operation was started because the search field was focused out (because hitting ENTER). Rescoped the ticket so only clicking the filter apply button or hitting ENTER will apply the new filters.
You didn't see #20 to be resolved because you were expecting a message even if there were no changes done to the translation - now I also show a status message saying this.
- π·π΄Romania claudiu.cristea Arad π·π΄
I've applied myself few minor changes and created three follow-ups:
- β¨ Ajaxify the translation form filters Active
- β¨ Refreshing the page, all filters and current page state are lost Active
- β¨ After applying a filter move to the 1st page Active
From my point of view this is RTBC
- π·π΄Romania claudiu.cristea Arad π·π΄
Thank you for feedback on my changes
-
claudiu.cristea β
committed 18750667 on 1.x authored by
huzooka β
Issue #3533685 by huzooka, claudiu.cristea, alorenc: Refactor...
-
claudiu.cristea β
committed 18750667 on 1.x authored by
huzooka β