Website translation

[share-with-me]

No description provided.

[sushain97]

Please try to take a critical eye towards your own code! I can only spend a certain amount of time looking at this per day so we need to cut down the number of times I provide feedback that results in changes being required.

[sushain97]

Doing it this way is really hacky and prone to error. We have getURLParam and HASH_URL_MAP for a reason.

[sushain97]

In fact, right now translation gets triggered automatically on page load if there's text present, ideally we could easily modify that code to make it also do this.

[share-with-me]

In fact, right now translation gets triggered automatically on page load if there's text present, ideally we could easily modify that code to make it also do this.

It gets fired even if there's no text present. I believe I have figured out what causes the webpage translation to not fire up on page refresh. There is translate() call in handlePairs() method in translator.js file which gets fired on page refresh (and this causes the translation request to be executed when on text interface). When on webpage translation interface, on page refresh, this translate() gets called even before showTranslateWebpageInterface() completes and thus, on every page refresh, the if condition gets satisfied for translateText div being visible and translateText() gets called.

In the below image, even when we are on webpage interface, the translate() method calls the branch corresponding to translateText().

I added a timeout to the translate() call in the handlePairs() method and it successfully executed the webpage translation in the event of page refresh when on webpage translation interface (because by this time, the webpage interface had appeared completely!)

A solution I can think of is:
I check the hash of the current page and if it is for webpageTranslation, I could defer the call of translate() till showWebpageTranslateInterface() gets completed? If I choose this method, how could I keep a track of order, because both the methods are running independently and I am not calling showTranslateWebpageInterface() from handlePairs where I could simply wait for the former to get over.

Or, I could check for the hash of current page, call the translateWebpage() method explicitly on page load but I'd make use of HASH_URL_MAP and related variables to achieve this ?

[sushain97]

You could call translate or the appropriate function at the end of showTranslateWebpageInterface? Also, can we fix the other problem like this:

diff --git a/assets/js/translator.js b/assets/js/translator.js
index a4f346d..d0ca047 100644
--- a/assets/js/translator.js
+++ b/assets/js/translator.js
@@ -392,9 +392,12 @@ function getPairs() {
         if(!pairData) {
             populateTranslationList();
             restoreChoices('translator');
-            translate();
+            if($('#originalText').val()) {
+                translate();
+            }
             return;
         }
+
         $.each(pairData, function (i, pair) {
             if(config.ALLOWED_PAIRS && config.ALLOWED_PAIRS.indexOf(pair.sourceLanguage + '-' + pair.targetLanguage) === -1) {
                 return;
@@ -425,7 +428,9 @@ function getPairs() {

         populateTranslationList();
         restoreChoices('translator');
-        translate();
+        if($('#originalText').val()) {
+            translate();
+        }
     }

     return deferred.promise();

or:

diff --git a/assets/js/translator.js b/assets/js/translator.js
index a4f346d..a40badd 100644
--- a/assets/js/translator.js
+++ b/assets/js/translator.js
@@ -392,9 +392,10 @@ function getPairs() {
         if(!pairData) {
             populateTranslationList();
             restoreChoices('translator');
-            translate();
+            translate(true);
             return;
         }
+
         $.each(pairData, function (i, pair) {
             if(config.ALLOWED_PAIRS && config.ALLOWED_PAIRS.indexOf(pair.sourceLanguage + '-' + pair.targetLanguage) === -1) {
                 return;
@@ -425,7 +426,7 @@ function getPairs() {

         populateTranslationList();
         restoreChoices('translator');
-        translate();
+        translate(true);
     }

     return deferred.promise();
@@ -624,19 +625,19 @@ function populateTranslationList() {
     }
 }

-function translate() {
+function translate(ignoreIfEmpty) {
     if($('div#translateWebpage').is(':visible') || isURL($('#originalText').val())) {
         translateWebpage();
     }
     else if($('div#translateText').is(':visible')) {
-        translateText();
+        translateText(ignoreIfEmpty);
     }
     else if($('div#docTranslation').is(':visible')) {
         translateDoc();
     }
 }

-function translateText() {
+function translateText(ignoreIfEmpty) {
     function handleTranslateSuccessResponse(data) {
         if(data.responseStatus === HTTP_OK_CODE) {
             $('#translatedText').val(data.responseData.translatedText);
@@ -647,9 +648,15 @@ function translateText() {
         }
     }

+    var originalText = $('#originalText').val();
+
+    if(!originalText && ignoreIfEmpty) {
+        return;
+    }
+
     if($('div#translateText').is(':visible')) {
         if(pairs[curSrcLang] && pairs[curSrcLang].indexOf(curDstLang) !== -1) {
-            sendEvent('translator', 'translate', curSrcLang + '-' + curDstLang, $('#originalText').val().length);
+            sendEvent('translator', 'translate', curSrcLang + '-' + curDstLang, originalText.length);

             if(translateRequest) {
                 translateRequest.abort();
@@ -666,7 +673,7 @@ function translateText() {
                 request = {'langpair': curSrcLang + '|' + curDstLang};
             }

-            request.q = $('#originalText').val(); // eslint-disable-line id-length
+            request.q = originalText; // eslint-disable-line id-length
             request.markUnknown = $('#markUnknown').prop('checked') ? 'yes' : 'no';
             translateRequest = callApy({
                 data: request,

[share-with-me]

@sushain97 . I have a question here in the following snippet:

if(!originalText && ignoreIfEmpty) {

Would it be a right behaviour to not at all make the text translate request even when only originalText is empty. ie the condition being if(!originalText) { return; }' ? translateText() gets called from a lot of methods and having this check in the method itself wouldn't require ignoreIfEmpty. IMHO, clicking on Translate button when the input originalText is empty shouldn't execute the request.

[sushain97]

Would it be a right behaviour to not at all make the text translate request even when only originalText is empty

No. Then if a user clears out the original text field, the result would not change. If we've made the other modes behave like that, it's buggy and should be fixed. APy should be fixed to respond to blank requests properly.

[share-with-me]

Oh, yes! I shall update this accordingly :D

[sushain97]

Leave this here since master has it as well. No need to remove it.

[share-with-me]

Please try to take a critical eye towards your own code! I can only spend a certain amount of time looking at this per day so we need to cut down the number of times I provide feedback that results in changes being required.

I am really sorry for this @sushain97 . I guess I was just being a bit impatient (and excited!) to see this getting merged (as it shall lead to version upgrade :D) ! I'd make sure to refactor well before making changes. I shall be more meticulous with the next submissions. Apologies for the inconvenience.

I really want to see this getting merged. I can't wait!

[share-with-me]

@sushain97 , I have made changes as you had suggested. I have been more meticulous this time and am really sorry if something is not completely correct. I had observed one more functionality flaw ie. when the user is on webpage translation interface and then navigates to Analyzer, or so forth and then switches back to Translation container, the webpage interface used to appear but the hash being of #translation. Thus, I also change the href attribute dynamically now depending on the interface button being clicked.

Added one check where if the originalText is empty, and translate webpage button is clicked, translateWebpage does not get called.

I have been trying to change the CSS and so far, I am unable to . I shall keep working on it. I updated this PR so that you could have a look at rest of the functionality. Thanks.

[share-with-me]

@sushain97 , I have been trying to fix the CSS issue. I have googled a lot with my search ranging from div filling up space between 2 fixed buttons, right div fixed and left div occupies the rest of the available space use of float, overflow, wrap to usage of btn-group, btn-group-justified in bootstrap, and so forth but I could not come up with a working solution using these. A lot of solutions made use of float and overflow: auto.

I, however, have coded a solution that doesn't use explicit hardcoded values of widths. I am assigning col-sm-* classes to srcLangSelectors on different interfaces. It does seem to display correctly. I'd push the code to this PR for you to review.

As this is the only issue that is pending with respect to website translation, I'd appreciate any help you could provide with respect to other CSS solutions :D. Really excited to see this getting merged!

[share-with-me]

@sushain97 , got rid of the .srcLangSelectorsWebpageTranslation. I did it in a different way where I apply the necessary CSS on cancelTranslateWebpage button instead.