Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

Crowdin in-context translation integration #773

Merged
merged 1 commit into from
Jan 20, 2017

Conversation

bdunogier
Copy link
Member

@bdunogier bdunogier commented Jan 4, 2017

EZP-26828

Adds the javascript code that integrates Crowdin's in-context translation UI.

In-context translation of PlatformUI

Usage

To trigger translations, the ach-UG translation, specific to crowdin, must be installed. It can be done by running composer require ezplatform-i18n/ezplatform-i18n_ach_ug. The requirement will be added to ezsystems/ezplatform by default.

To trigger the in-context UI, ezsystems/ezpublish-kernel#1889 is required.

TODO

  • BazingaTranslationBundle patch (Accept three letters languages willdurand/BazingaJsTranslationBundle#191)
  • Clarify where the ach-UG translation files should be taken from / hosted (added instructions + downloadable file)
  • Behat scenario ?
  • Unit test the subscriber
  • Consider changing the subscriber's folder (talk to Nicolas)
  • Make the name of the crowdin project configurable (or follow-up ?).
    Would allow any project to use in-context translation.

@bdunogier bdunogier force-pushed the ezp26828-trigger_crowdin_in_context_translation branch from e779271 to e02560e Compare January 4, 2017 13:55
@bdunogier bdunogier changed the title EZP-26828: Trigger crowdin in-context translation based on a cookie WIP: Trigger crowdin in-context translation based on a cookie Jan 4, 2017
@bdunogier bdunogier force-pushed the ezp26828-trigger_crowdin_in_context_translation branch from af5ee0e to 590075d Compare January 6, 2017 11:08
@nicolas-bastien
Copy link
Contributor

@bdunogier for that point :

Clarify where the ach-UG translation files should be taken from / hosted (added instructions + downloadable file)

I guess it will be distribute in https://github.com/ezplatform-i18n/ezplatform-i18n-ach_UG. ?

@nicolas-bastien nicolas-bastien self-requested a review January 12, 2017 15:18
@bdunogier
Copy link
Member Author

bdunogier commented Jan 12, 2017

I guess it will be distribute in https://github.com/ezplatform-i18n/ezplatform-i18n-ach_UG. ?

It would work, yes.

But my issue with that is that you would have to install something else in order to get working a feature that is supposed to work. In-context will just not work without it. I'd rather have it shipped with the product, so that anybody can translate.

@bdunogier
Copy link
Member Author

About the open question above, my plan turns out to be a little more complex:

Given that in-context has been proven to work here, we could:

  • move the cookie listener to the CoreBundle.
  • keep the shell.html.twig changes in this pull-request and merge them. They're totally harmless unless you trigger the custom language
  • add ezplatform-i18n/ezplatform-i18n-ach_ug to the ezplatform requirements. That way, users are free to remove it (note that we MUST tell people to add it to their own composer.json)

Any opinion @dpobel or @nicolas-bastien ?

@nicolas-bastien
Copy link
Contributor

@bdunogier

  • Moving the listener to core make sense cause translation collector is already there
  • I agree for the two other points

@bdunogier bdunogier changed the title WIP: Trigger crowdin in-context translation based on a cookie Crowdin in-context translation integration Jan 19, 2017
@bdunogier
Copy link
Member Author

Cleanup done !

@bdunogier bdunogier force-pushed the ezp26828-trigger_crowdin_in_context_translation branch from 81a5bbe to df28f8c Compare January 19, 2017 11:02
@bdunogier
Copy link
Member Author

Ping @yannickroger @nicolas-bastien (for the cleaned up version).

@bdunogier bdunogier merged commit 3297f26 into master Jan 20, 2017
@bdunogier bdunogier deleted the ezp26828-trigger_crowdin_in_context_translation branch January 20, 2017 00:22
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants