Bug 9017 - Quote of the day: Table footer not translated
Summary: Quote of the day: Table footer not translated
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: 3.10
Hardware: All All
: P5 - low normal (vote)
Assignee: Marc Véron
QA Contact: Paul Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-07 10:16 UTC by Marc Véron
Modified: 2013-12-05 20:06 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
QOD table footer not translated (208.26 KB, image/jpeg)
2012-11-07 10:16 UTC, Marc Véron
Details
Bug 9017 - Quote of the day: Table footer not translated (3.07 KB, patch)
2012-11-07 13:01 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 9017 - Quote of the day: Table footer not translated (3.09 KB, patch)
2012-11-11 06:52 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9017 - Quote of the day: Table footer not translated (3.14 KB, patch)
2012-11-20 12:38 UTC, Paul Poulain
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Véron 2012-11-07 10:16:09 UTC
Created attachment 13277 [details]
QOD table footer not translated

The strings for the table footer of the QOD feature are translated in /datatables-strings.inc, but they do not show up in staff client due to missing code in quotes.tt -> $(document).ready(function(), part "oLanguage"
See: http://datatables.net/usage/i18n

Additionally, quotes.tt has a stray </script> tag after [% INCLUDE 'datatables-strings.inc' %]

Patch follows.
Comment 1 Marc Véron 2012-11-07 13:01:06 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2012-11-07 13:03:03 UTC
Would be great to have it in 3.10 because translation is broken. (No string change involved.)
Comment 3 Owen Leonard 2012-11-07 20:53:04 UTC
I don't think this is the correct approach. If you look at members/readingrec.tt you can see that the table footer *is* translated. The difference is that the initialization of the table sorter on that page includes a directive to extend according to some defaults:

    $("#table_readingrec").dataTable($.extend(true, {}, dataTablesDefaults, {
        "sPaginationType": "four_button",
    }));

dataTablesDefaults loads the configuration in datatables.js which loads the variables defined in datatables-strings.inc. And datatables-strings.inc exists expressly to be picked up for translation, so duplicating its contents shouldn't be necessary.

That said, simply adding $.extend() to the datatables configuration in quotes.tt doesn't work for me and I'm not sure why.
Comment 4 Marc Véron 2012-11-07 22:27:09 UTC
In this case, the architecture the code is fundamentally wrong and should be re-written?

Who would have time to do it? 

And in the meantime we have ugly non-translated text in localized versions I wanted to get rid of.
Comment 5 Marc Véron 2012-11-08 12:02:44 UTC
I tried for several hours but I can't make it work with the approach Owen suggested. It seems that the table definition is to complex to be merged with dataTablesDefaults.

If I extend it with:
    oTable = $("#table_readingrec").dataTable($.extend(true, {}, dataTablesDefaults, {
...
    }));

...the table simply does not render. I suppose that it breaks the callback definitions. 

My patch would be a workaround to avoid untranslated strings and does not break anything, but maybe somebody else (the original author of this feature?) is able to put the code in a way that it does not fail QA.
Comment 6 Chris Nighswonger 2012-11-09 15:12:31 UTC
(In reply to comment #5)
> I tried for several hours but I can't make it work with the approach Owen
> suggested. It seems that the table definition is to complex to be merged
> with dataTablesDefaults.

This is true. dataTablesDefaults seemed to impose some restrictions which I was unable to workaround. And I seem to recall not being able to modify the extension to render it usable for my purposes.

> 
> My patch would be a workaround to avoid untranslated strings and does not
> break anything, but maybe somebody else (the original author of this
> feature?) is able to put the code in a way that it does not fail QA.

I think this is a good workaround. It uses the same basic approach as the extension and one which is (iirc) recommended in the DT documentation. It would be nice to explore the idea of separating the strings from dataTablesDefaults, allowing perhaps a more benign extension of DT to just include strings.

Changing the status back to "Needs Signoff." I don't have multiple translations installed atm, so if someone who does could verify and sign-off, that would be good.
Comment 7 Chris Cormack 2012-11-11 06:52:23 UTC Comment hidden (obsolete)
Comment 8 Paul Poulain 2012-11-20 12:37:59 UTC
Qa comment:
 * small patch, no side effect
 * make strings translatables
passed QA
Comment 9 Paul Poulain 2012-11-20 12:38:10 UTC
Created attachment 13550 [details] [review]
Bug 9017 - Quote of the day: Table footer not translated

The strings for the table footer of the QOD feature are translated in /datatables-strings.inc, but they did not show up in staff client due to missing code in quotes.tt -> $(document).ready(function(), part "oLanguage"
Code added as appropriate.
See: http://datatables.net/usage/i18n

Additionally, quotes.tt had a stray </script> tag after [% INCLUDE 'datatables-strings.inc' %]

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Comment 10 Jared Camins-Esakov 2012-11-20 19:21:57 UTC
This patch has been pushed to master.
Comment 11 Paul Poulain 2012-11-22 15:38:26 UTC
Patch pushed to branch 3.10.x
Comment 12 Chris Cormack 2013-04-23 10:17:04 UTC
Released in 3.10.0