Skip to content
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

Translation string replacement errors #7798

Closed
jywarren opened this issue Apr 8, 2020 · 28 comments · Fixed by #9696
Closed

Translation string replacement errors #7798

jywarren opened this issue Apr 8, 2020 · 28 comments · Fixed by #9696
Labels
bug the issue is regarding one of our programs which faces problems when a certain task is executed fto-candidate issues which are meant to be solved by first timers but aren't well-formatted yet help wanted requires help by anyone willing to contribute high-priority Translation System issues which involve adding new translations, improving the translation system

Comments

@jywarren
Copy link
Member

jywarren commented Apr 8, 2020

A huge and wonderful effort to set up translation strings for site-wide texts has been going on for some time in #7416 and other issues -- many thanks to those who've helped!

I noticed that when a non-english language is selected, we now have some issues with the translation icons appearing:

image

Some are parsing errors causing for example the text " data-placement="bottom" href="/dashboard">Dashboard to appear. Others simply add the globe icon causing items not to fit in the layout anymore. For comparison, here is the dashboard in English:

image

You can see this happening by clicking on this link which will change the language of the stable server so you can see the translations turned on (this for spanish):

https://stable.publiclab.org/change_locale/es

I'd like to ask for help in solving these parsing and layout issues! They should be pretty simple and you should be able to test them locally by setting a language locally, for example with:

http://localhost:3000/change_locale/es

Thanks, everyone!

@jywarren jywarren added bug the issue is regarding one of our programs which faces problems when a certain task is executed high-priority help wanted requires help by anyone willing to contribute fto-candidate issues which are meant to be solved by first timers but aren't well-formatted yet Translation System issues which involve adding new translations, improving the translation system labels Apr 8, 2020
@jywarren
Copy link
Member Author

jywarren commented Apr 8, 2020

Many of these translations were added starting from this issue, for reference! #6579

@jywarren
Copy link
Member Author

jywarren commented Apr 8, 2020

I believe that the switch to the custom translation function has caused a parsing issue in some cases.

Here is documentation on how this process is supposed to work, via @gauravano (hiiii!!! 👋 ❤️ hope you're well!!!)

#5737 (comment)

Hi @jywarren @nstjean, yeah, t(..) can be replaced by translation(...). The code for translation is in helpers/application_helper.rb.

NOTE: After replacing, testing that view is necessary as it may not work for complex blocks like div, span, etc.

And, if corner cases need to be handled, then one can change the helper definition.

@jywarren
Copy link
Member Author

jywarren commented Apr 8, 2020

cc @cesswairimu @SidharthBansal @nstjean just as a heads up! Thanks to everyone!

@jywarren

This comment has been minimized.

@jywarren jywarren closed this as completed Apr 8, 2020
@jywarren
Copy link
Member Author

jywarren commented Apr 8, 2020

Ah sorry! Wrong issue! This is still open!

@jywarren jywarren reopened this Apr 8, 2020
@cesswairimu
Copy link
Collaborator

cesswairimu commented Apr 8, 2020

oh that looks ugly,..I will do some investigating tomorrow

@Tlazypanda
Copy link
Collaborator

@jywarren Had a look apart from the error that you already fixed can't seem to find any more buggy routes 😅 . I checked by seeing the recent translation codes merged and checking those specific routes. If there is any particular route that has bugs I would be happy to help 😄

@jywarren
Copy link
Member Author

Hi @Tlazypanda i think the routes issue should be all good now? That was separate from this, and i think i just had them mixed up with too many tabs. Does this translation bug make sense though? Thanks!

@cesswairimu
Copy link
Collaborator

@jywarren did you manage to fix this...I am looking at stable and I don't see the bugs

@jywarren
Copy link
Member Author

i don't think so - to reproduce you have to select a non-english language using the footer selector

@jywarren
Copy link
Member Author

So for the navbar, which is the worst issue, I see the HTML resulting from the adapted translation code is as follows:

<div class="input-group">
          <input type="text" id="searchform_input" class="form-control search-query typeahead" role="search" qrytype="tags" placeholder="<span>Search on Public Lab <a href=" https:="" www.transifex.com="" publiclab="" publiclaborg="" translate="" #de="" $?q="text%3ASearch" on="" public="" lab"="" style="cursor: text;">
          <i data-toggle="tooltip" data-placement="top" title="" style="position:relative; right:2px; color:#bbb; font-size: 15px;" class="fa fa-globe" data-original-title="Needs translation? Click to help translate this text."></i>
       "  value="" required&gt;
          <div class="input-group-append">
            <button class="btn btn-light" type="submit"><i class="fa fa-search"></i></button>
          </div>
        </div>

So, it's not nesting properly and looks like this:

image

@jywarren
Copy link
Member Author

jywarren commented Apr 14, 2020

For that one (let's do other fixes on other issues but this walks us through the process of one particular fix, as an example), I see line 20 shows there's only one change, to the placeholder attribute of the search:

<div class="collapse navbar-collapse" id="header-navbar-collapse">
<form class="form-inline mr-3" id="searchform" autocomplete="off">
<div class="input-group">
<input type="text" id="searchform_input" class="form-control search-query typeahead" role="search" qryType="tags" placeholder="<%= translation('layout._header.search') %>" value="<%= params[:query] %>" required>
<div class="input-group-append">
<button class="btn btn-light" type="submit"><i class="fa fa-search"></i></button>
</div>
</div>
</form>

Strangely the string that's supposed to be inserted there is pretty simple:

search: "Search on Public Lab"

I bet it's the translation application helper:

def translation(key, options = {})
translated_string = t(key, options)
options[:fallback] = false
translated_string2 = t(key, options)
if current_user&.has_tag('translation-helper') && translated_string2.include?("translation missing") && !translated_string.include?("<")
raw(%(<span>#{translated_string} <a href="https://www.transifex.com/publiclab/publiclaborg/translate/#de/$?q=text%3A#{translated_string}">
<i data-toggle='tooltip' data-placement='top' title='Needs translation? Click to help translate this text.' style='position:relative; right:2px; color:#bbb; font-size: 15px;' class='fa fa-globe'></i></a>
</span>))
else
raw(translated_string)
end
end

So here, we are essentially inserting HTML markup into a placeholder tag, which is already a problem, and then we are messing up the quotation mark nesting.

What we probably need to do here is to detect if we are within an HTML attribute and to NOT insert more HTML into such attributes.

The easiest thing would be to just revert the use of the translation helper in #7744, AND update guidance to say that we shouldn't use it within HTML attributes? cc @cesswairimu

@jywarren
Copy link
Member Author

There are also a lot of layout issues related to the globe icon appearing in the header navbar. I'm going to revert the bunch right now in #7815, and we may want to think about a different kind of indicator for use in those places.

jywarren added a commit that referenced this issue Apr 14, 2020
* revert translation change within HTML tag attributes

cc #7798

* Update _header.html.erb
@jywarren
Copy link
Member Author

Great, that fixed the navbar. Let's sum up next steps:

  • warnings about usage in buttons and HTML tag attributes
  • better guidance on how to test these locally and how to post screenshots of them working
  • any other follow-up fixes for bad layout issues resulting

Thanks!!

@jywarren
Copy link
Member Author

Just noting that there have also been a lot of these translation changes made to the login/signup form and that is potentially another area where they may have caused issues. However I can't seem to document these well as I can't switch languages in incognito mode... maybe i need to be logged in?

@Tlazypanda
Copy link
Collaborator

@jywarren I am not sure what's wrong but I am not able to reproduce any bugs... when I previously commented too I was referring to the navbar and it was not buggy (before you committed the fix) @cesswairimu did you face the same issue? I tried again with both stable/local, in incognito mode, setting various languages (hindi,spanish,dutch) can you point out where I am going wrong 😅

@Tlazypanda
Copy link
Collaborator

As for the buttons part I went through the code and these are the instances where translation is used inside a button so that we can test if these routes are bug free. For the warnings generation we can update the internationalization part in the docs to specify what @gauravano had mentioned in his comment earlier. Please help me with how to reproduce these bugs too 😅 . Thanks ✌️ -
Screenshot from 2020-04-17 00-38-27

@jywarren
Copy link
Member Author

jywarren commented May 6, 2020

Hi, sorry for slow response, thanks for the thorough look! I'm not sure why it's not appearing for you, but I agree about use inside buttons. I just filed a new instance that is exactly that!

#7882

@jywarren
Copy link
Member Author

Hi all, i've just created 2 FTOs with examples of this going wrong - check out the above links! In both cases, I left an explanatory note to future coders. Both are reproducible on the live site right now! Thanks @Tlazypanda and apologies for responding slowly! Also cc'ing @gauravano ❤️

@jywarren
Copy link
Member Author

Another instance i had to revert due to poor rendering -- #8565

And, noting a couple bad renders here at https://stable.publiclab.org/tag/water-quality -- (click https://stable.publiclab.org/change_locale/es first to turn on translations to see this)

image

@grvsachdeva
Copy link
Member

Hi @cesswairimu @Tlazypanda, you're not able to see the globe icons because your profile doesn't have translation-helper tag. It is the first condition checked in the code.

if current_user&.has_tag('translation-helper').

This wiki - https://publiclab.org/wiki/translation contains join the translation group button which is also doing the same thing, Adding the tag to the user profile.

I also checked out the documentation - https://github.com/publiclab/plots2#internationalization and I feel it is very much focussed on developers. Like, we can also provide link to Public Lab Transifex account and user can sign-up for being contributor there?

I saw some PRs opened for #6579 and looks like views are not tested. Maybe contributors were not aware of adding translation-helper tag. So, a point about this can be called out in the documentation. Or, we can provide Translator role from the Settings page?

Thanks!

cc @jywarren @ebarry

@grvsachdeva
Copy link
Member

grvsachdeva commented Oct 14, 2020

As of now only 5 users are Translators so the distorted view is not visible to general public 😄.

Screenshot 2020-10-14 at 7 22 40 PM

Next steps:

  1. We should compile a list of pages where view is distorted, and divide the rectification among ourselves.

  2. We don't need to add/use translation helper everywhere because there can be multiple instances of same text on a single page or all over the website. So, if one is resolved all the globe icons would be disabled so goal should be to enhance the Translator experience.

  3. A translator will not translate each string by clicking on globe icon one by one and would prefer to do multiple translations at Transifex so more feature discovery prompts should be there. Globe icon is one, another one is Language selector. We can also add a prompt whenever someone switches to non-English in case they're helping with Translating Public Lab and re-direct them to Transifex. [This point is highly opinionated]

Screenshot 2020-10-14 at 7 31 59 PM

I think @ebarry and @jywarren can help how much effort we want to put on Translation and what would be good user experience 😄

Thanks!!

@tawahpeggy
Copy link

i am intrested to work on this issue if possible.

@YogeshSharma01
Copy link

Hi, @grvsachdeva like only users who have an account on Transifex those users can only translating the website into different languages using Transifex. When I try to translate the text by clicking on the 'Get Started' in the globe icon, I'm directed to the 404 pages on the transfix website
Screenshot 2021-03-30 at 11 50 10 PM.
Also, I have contacted the support team but it takes so long to respond so can you help me with this I also open an issue for this #9394 this is the link no longer work.
What do you think the account expired or if they substantially changed the API?

@grvsachdeva
Copy link
Member

grvsachdeva commented Apr 4, 2021

Hi @YogeshSharma01, yeah account is required to act as a translator on Transifex, and apart from that I believe the project need to add person as a translator or in the relevant role too. I can see there are a bunch of requests pending to be accepted.

Hey @jywarren @ebarry please checkout https://www.transifex.com/publiclab/teams/65351/requests/ 😄

Thanks!

@YogeshSharma01
Copy link

Hi, @grvsachdeva you're right please accept my request.
Thanks

@ebarry
Copy link
Member

ebarry commented Apr 5, 2021

I just approved 4 people's requests in Transifex to localize in Hindi. I did not yet approve @YogeshSharma01 to localize in Spanish and Russian -- please let me know if you really speak those languages! thank you :))))

@YogeshSharma01
Copy link

Hi, @ebarry Thanks for approval. No no not really I only speak English and Hindi languages actually i was just trying to figure out how this transfix is really working so i requested for other languages too and sorry for that.
😁 Thanks for approved my request in Transfix to localize in hindi.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug the issue is regarding one of our programs which faces problems when a certain task is executed fto-candidate issues which are meant to be solved by first timers but aren't well-formatted yet help wanted requires help by anyone willing to contribute high-priority Translation System issues which involve adding new translations, improving the translation system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants