New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix translator, enable selected text translation #4407

Merged
merged 2 commits into from Dec 17, 2018

Conversation

Projects
None yet
3 participants
@poire-z
Copy link
Contributor

poire-z commented Dec 16, 2018

Update translator.lua to use a still working google translate API (while it works, it may break or be forbidden at any moment - calling that url with curl returns 403 forbidden...)
Add a method to show translations (main and alternates) in a TextViewer.
Re-enable "Translate" button in text selection/highlight buttons dialog (the big diff in the buttons setup is just because of indentation, I only replaced Follow link with Translate, and moved Follow link in a conditional below).
Target language can be set with a manually added setting translator_target_language = "fr" and will fallback to the UI language (we could have a Translation settings> below the Dict and Wikipedia ones if needed - but that would be for another PR). Added that menu as part of this PR.

image

To get a slot for it, I made the Follow link button being added only when it is needed:
image

image

image

@poire-z poire-z force-pushed the poire-z:fix_translator branch from 282fa73 to baedde0 Dec 16, 2018

Fix translator, enable selected text translation
Update translator.lua to use a still working google translate API.
Add a method to show translations (main and alternates) in
a TextViewer.
Re-enable "Translate" button in text selection/highlight buttons
dialog.
Target language can be set with a manually added setting:
    translator_target_language = "fr"
and will fallback to the UI language.

@poire-z poire-z force-pushed the poire-z:fix_translator branch from baedde0 to 71f0027 Dec 16, 2018

@poire-z

This comment has been minimized.

Copy link
Contributor

poire-z commented Dec 16, 2018

I'll probably add this Translation settings> menu tomorrow, just because I don't want to add that manual setting translator_target_language to #2589 (comment) that I previously tried to empty :)

Would these be the right english wordings: Set translation target language Set translation source language.

@AlanSP1

This comment has been minimized.

Copy link

AlanSP1 commented Dec 16, 2018

It would be enough to use Set target and source language, as you get idea from translate menu, in my opinion.

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Dec 16, 2018

Yes, that wording is fine. Depending on the specifics perhaps the simpler Translate to and Translate from could work?

Edit: or as, Translate to: %1 with a submenu.

adds "Translation settings" menu
and the list of 104 languages supported by Google Translate
@poire-z

This comment has been minimized.

Copy link
Contributor

poire-z commented Dec 17, 2018

Thanks, Translate from/to: sounds nice and obvious.

image

image

image

(I put the 104 language names in _("") if one wants to translate them - dunno if that will flood transiflex'ers.)
(The languages are ordered by language code - I hesitated giving some "level" to them to make the most popular ones line English, Portuguese... appear first - but who am I to judge ? so I decided to not discriminate against minorities so we avoid getting some buzz on twitter with a worldwide #MyLanguageToo wave !)

Also removed the target language name in the title, to make room for larger source language names:
image

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Dec 17, 2018

dunno if that will flood transiflex'ers.)

It would, but personally I'll just do a couple a day. :-P

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Dec 17, 2018

I'll add a note on Transifex that they can all be found pre-translated here: https://nl.wikipedia.org/wiki/Lijst_van_ISO_639-codes (or rather, on the relevant Wikipedia translation)

@Frenzie Frenzie merged commit 80cb9d0 into koreader:master Dec 17, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@poire-z poire-z deleted the poire-z:fix_translator branch Dec 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment