Skip to content
This repository has been archived by the owner on Jan 10, 2023. It is now read-only.

FEATURE: Translate CR Nodes automatically. #1

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mficzel
Copy link
Contributor

@mficzel mficzel commented Sep 28, 2021

Translate node properties while they are adopted to another language.

When nodes are copied (adopted) into another languge the fields can be translated automatically.

The following setting enables the translation of all inlineEditable properties:

CodeQ:
  DeepLTranslationHelper:
    nodeTranslations:
      translateInlineEditables: true

Other properties of type string can be translated aswell with the following configuration.

Neos.Neos:Document:
   properties:
     title:
       options:
         deeplTranslate: true

In additions this PR:

  • moves the caching to the eel helper
  • implements the deepl service with psr factories and request to avoid an explicit dependency to guzzle
  • adds a Setting CodeQ.DeepLTranslationHelper.DeepLService.useFreeApi: false to switch between free and payed api
  • adds a Setting CodeQ.DeepLTranslationHelper.DeepLService.defaultOptions to configure additional arguments for the translation

This has still lots of limitations an probably needs tweaking.
@mficzel mficzel changed the title FEATURE: Translate CR Nodes on create. FEATURE: Translate CR Nodes automatically. Sep 28, 2021
@rolandschuetz
Copy link
Contributor

Wow, that was fast :)

Definitely needs some docs.
Let me know if I can help.

@mficzel
Copy link
Contributor Author

mficzel commented Sep 28, 2021

Am bit skeptical regarding performance because every translated property will yield a separate request.

Maybe the documents api could help here as this would support to translate multiple fields at once.

Or we trick a little and put all the properties of a node into a single html that gets translated and later split again. However would render the caching quite obsolete for this specific use case.

This is in preparation of translating multiple texts with one api call in the deepl service.
@mficzel mficzel force-pushed the feature/translateContentRepsitoryNodes branch from 28ce5c7 to eac360d Compare September 29, 2021 20:22
@mficzel
Copy link
Contributor Author

mficzel commented Sep 30, 2021

@rolandschuetz some things that i still would like to improve are

  • Some explicit mapping of dimension values to source and target language. Currently the first part of the language dimension value
  • Some explicit configurations for glossaries (or probably additional request argument options based on language combination)
  • Do nothing when no language dimension exists or make the whole dimension configurable.

Do you have specific ideas for that?

PS: If you feel this totally is out of scope for your package just tell me and i will use the code otherwise. Could make sense to have a separate TranslationHelper and NodeTranslator package.

This avoids the direct dependency to guzzle but still uses psr factories and requests as much as possible.
Only the http timeout is set as curl option.
@mficzel mficzel marked this pull request as ready for review October 20, 2021 13:15
@mficzel
Copy link
Contributor Author

mficzel commented Nov 5, 2021

Close in favor of custom package

@mficzel mficzel closed this Nov 5, 2021
@rolandschuetz
Copy link
Contributor

@mficzel Sorry for the late response, I thought we might go over it together next week.
Was the delay for merging to long for you, or is your package structure very different?

@mficzel
Copy link
Contributor Author

mficzel commented Nov 5, 2021

The new package has very few similarities to yours and even my original pr:
https://github.com/sitegeist/Sitegeist.LostInTranslation
Since this package also offers totally different features than yours this allows us to move faster independently.

@rolandschuetz
Copy link
Contributor

Looks cool.

So I will keep my package as EelHelper.. what I need, and keep your project in mind if I need that feature in another project :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants