Convert locale theme tables to table #type

Taking a crack, one of these will go through

Got nowhere slowly... going to give up on this one, first thought though was doesn't need #type=>table because it doesn't need tableselect or tabledrag... but that seems to take it out of the form_api and none of the textarea's get #value properties.

#3: 1938916-3-locale-tables.patch queued for re-testing.

Tagging.

Profiling data

• Two run
• 500 iterations
• http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519feaa2b1af9&...

admin/config/regional/translate

=== 8.x..8.x compared (519feaa2b1af9..519fee1c89203):

ct  : 60,623|60,623|0|0.0%
wt  : 284,555|284,692|137|0.0%
cpu : 260,739|261,023|284|0.1%
mu  : 12,707,864|12,707,864|0|0.0%
pmu : 12,902,160|12,902,160|0|0.0%

=== 8.x..twig-locale-table-1938916 compared (519feaa2b1af9..519feedd48170):

ct  : 60,623|58,138|-2,485|-4.1%
wt  : 284,555|276,055|-8,500|-3.0%
cpu : 260,739|251,918|-8,821|-3.4%
mu  : 12,707,864|12,543,984|-163,880|-1.3%
pmu : 12,902,160|12,793,704|-108,456|-0.8%

ct = function calls, wt = wall time, cpu = cpu time used, mu = memory usage, pmu = peak memory usage

The profiling looks great. Though there is a markup issue that needs to be addressed.

+++ b/core/modules/locale/locale.pages.inc
@@ -692,48 +694,6 @@ function _locale_translation_status_debug_info($source) {
-  $output .= theme('pager');

Also missing pager

Rerolled patch with added pager attached.

10: drupal-convert_locale_theme_table-1938916-10.patch queued for re-testing.

Tests pass locally, retesting. Also needs another round of markup and performance testing because the theme function was removed though not used.

10: drupal-convert_locale_theme_table-1938916-10.patch queued for re-testing.

Rerolled

@akalata, here's a code review.

+++ b/core/modules/locale/src/Form/TranslateEditForm.php
@@ -44,10 +44,17 @@ public function buildForm(array $form, FormStateInterface $form_state) {
+        t('Source string'),

Per Joel's previous suggestions, please use the updated array [] syntax and injected $this->t() in the #header array.

Regarding manually testing, I had to add a language at /admin/config/regional/language/add, then I could see the table at /admin/config/regional/translate.

I've attached an image of where it breaks. I think it's the removed lines 65 and 70, which include
and tags that need to be fixed. @joelpittet may have suggestions if you have questions.

Thanks @a-fro!

Attached an update after working with @joelpittet. We also noticed that the $string->context isn't being saved into the database -- maybe that part is blocked by #1035716: [policy, no patch] Decide and document guidelines for using string context?

@joelpittet and @a-fro saw that I'd commented a conditional to test $string->comment; adding that back!

This doesn't need profiling it's admin facing theme function conversions. I believe manual testing on this patch is all that is needed as everything seems to look good for me.

This patch is just minor nitpicky formatting that I hope is not too much but tried to minimize the patch diff by not changing everything.

Manual testing results were that the id on the table is still removed (expected and desired), xhtml br tags are now html5 (without closing slash) and the strings id's of the singular/plural have been replaced with indexes. @joelpittet said that is not a problem because they are not being targeted by css or js. Marking as rtbc.

Following up on my $string->context question from #20, I've found that a small number of imported translations have context data. I'm assuming they all should, but confirming that this is a different issue than this one.

+++ b/core/modules/locale/src/Form/TranslateEditForm.php
@@ -76,31 +78,37 @@ public function buildForm(array $form, FormStateInterface $form_state) {
+            '#template' => '<br /><small>{{ context_title }}: <span lang="en">{{ context }}</span></small>',

The br should not have a closing slash.

xhtml br tags are now html5 (without closing slash)

Thanks for catching that, saves me from doing that again in the self-closing #1388926: Remove all references to "self-closing" void elements in core

Tested this manually and didn't find any visual changes. Code looks also good to me so lets go.

This issue is a normal task so we need to outline how it fits within the allowable Drupal 8 beta criteria. Can someone add Drupal 8 beta phase evaluation template to the issue summary.

Everything looks still good and patch applies. I also added the beta evaluation.

Locale tables after patch:

Committed d4e8abd and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation for to the issue summary.