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

Place multiple translations into one banner #1738 #1749

Closed
wants to merge 4 commits into from

Conversation

amsichani
Copy link
Contributor

This is mainly a layout issue #1738 - put all of the various translation links of a lesson into a single banner.

Let's wait before we merged it, as its linked to the second part of #1738 : a language accessibility perspective "message in x language; title in x lang." I am also working on it and will make a different PR asap.

Put all of the various translation links into a single banner.
@amsichani amsichani self-assigned this Apr 22, 2020
@amsichani
Copy link
Contributor Author

I have now edit also the code in order to change the language of the messages (source lesson and translated lesson) in the landing pages. We decided not to change the snippets but instead to slightly amend the syntax of the messages -lets see if it works!

@jenniferisasi
Copy link
Contributor

jenniferisasi commented Apr 22, 2020

Screen Shot 2020-04-22 at 10 08 20 AM

Right: the translations show in one single banner now. Wrong: the message is showing twice; the name of the language is in English.

Also, now the message of the original source is not showing on the translated lessons:

Screen Shot 2020-04-22 at 11 31 39 AM

Also, I think you deleted the banner for donations?

Will look at it again with this issues in mind.

@jenniferisasi
Copy link
Contributor

jenniferisasi commented Apr 22, 2020

Comment in progress while I solve the issues to avoid doing multiple individual changes and using it as a learning process.

  1. The changes in the order of the first lines deleted the banner for the original lesson. This later includes:

  2. For the language of the original to show on the sentence, this is the original code:

{{ site.data.snippets.language-name.en[page.lang]}
changed to
{{ site.data.snippets.language-name[source.lang]}

Because in the snippets, language-name has further nesting, we still need to indicate the additional step of the language, in the original en because we only have originals in English right now (so would need to change it further later, am I right here @mdlincoln? [I only need confirmation on this one, while we try to solve the rest of the code])

@acrymble
Copy link

Thanks for this change and for the work you've put into it.

Just thinking ahead a little, this might eventually get quite long (3 possible translations per lesson). Are there visual ways we might consider to make it a bit less text heavy, with that future in mind? I'm thinking using the ES FR EN icons perhaps, but defer to your expertise.

@jenniferisasi
Copy link
Contributor

Right @acrymble, it would eventually have any combination of the following:
The original lesson was published in English: title.
Esta lección está traducida al español: título.
Cette leçon a été traduite en français: title
Esta lição é traduzida para o português: título

When a lesson is translated, the user can switch language already using the EN|ES|FR bar above... but with no indication of availability there. Maybe a banner with something like:

Original: EN; traducida: ES; traduite: FR; traduzida: PT

It's missing the titles, which was a nice touch.

Before I procede with any changes now, @amsichani @programminghistorian/global-team what do you think?
I don't anticipate this being a terrible technical change, but we are trying not to make too many; this would need a new snippet (actually, a change) and instead of the title of the lesson a link to EN, ES...but that links to the lesson itself (this I wouldn't know how to do)

1. Put back the donation banner. 
2. change the div tags to attempt to have only one banner for originals and translations. 
3. deleted a duplicate that was duplicating messages in banners
Attempt at fixing missing banner and non-matching languages
@mdlincoln mdlincoln marked this pull request as draft April 22, 2020 20:24
@jenniferisasi
Copy link
Contributor

Some success:

In the English version, we have the target languages
Screen Shot 2020-04-22 at 4 12 18 PM

But is not changing to the language of the original lesson:

Screen Shot 2020-04-22 at 4 14 37 PM

Screen Shot 2020-04-22 at 4 15 24 PM

And to get back the banner we lost, I clearly restored individual banners [hand-on-face-emoji]

@mdlincoln
Copy link
Contributor

mdlincoln commented Apr 22, 2020

Hm I am a little confused reading the comments here what the end goal is?

Could you draw a little diagram, maybe even a box like this:

---------------------
| DONATION BANNER 
---------------------


---------------------
| Esta lección ha sido traducida al español: Análisis de corpus con AntConc
|                                                                                                                      
| Cette leçon a été traduite en français: Analyse de corpus avec AntConc  
---------------------

to explain your end goal layout?

Then it will be easier to figure out how to edit the original template.

@jenniferisasi
Copy link
Contributor

@mdlincoln yes, let me try - and thanks for checking on us:

On the original lesson:

---------------------
| DONATION BANNER 
---------------------

---------------------
| Esta lección ha sido traducida al español: Análisis de corpus con AntConc
|                                                                                                                      
| Cette leçon a été traduite en français: Analyse de corpus avec AntConc  
---------------------

On a translated lesson:

---------------------
| DONATION BANNER 
---------------------

---------------------
| This lesson was originally published in English: Corpus Analysis with Antconc
|                                                                                                                      
| Cette leçon a été traduite en français: Analyse de corpus avec AntConc  
---------------------

Makes better sense?

@mdlincoln
Copy link
Contributor

Cool. First, get the html working again so everything is in one banner. Then: remember that page.lang is the language you're currently on, while candidate.lang is the language of the translated page you're linking to. Looks like the change you're trying to make is to have the "This lesson is translated from / into" in the candidate language, rather than the page language, which is what it was originally set to.

If you need to close this PR and start over from the base code, that would be preferable to trying to re-commit it.

@amsichani
Copy link
Contributor Author

sorry I ve missed the late night (for me!) party! thanks @mdlincoln for your magic touch ! As far as I can see @jenniferisasi you ve already made the changes- the only bit we need to amend is to include all the translations into one banner - is that right?

@jenniferisasi
Copy link
Contributor

@mdlincoln yes, exactly. And as you can see, I/we changed in the {% for candidate in translation_candidates %} the [page.lang] to [candidate.lang] - so this changes the languages of the sentences to the translated versions of a lesson.

Now, would that also work on for the original or {% unless page == translation_source %}? The fact that it is pointing to {{ site.data.snippets.language-name.**en**[page.lang]}} confused me a bit, in this sense that it has an additional nesting in there. I will try in the next iteration.

@amsichani this is the one change missing, to have the sentence about the original language of the lesson in its original language; then yes, only the html to put all in a single banner.

@mdlincoln
Copy link
Contributor

mdlincoln commented Apr 23, 2020

As I noted in #1751 that hardcoded .en is definitely an oversight - you can call the destination page language with site.data.snippets.language-name[translation_source.lang][translation_source.lang]

@mdlincoln
Copy link
Contributor

Following up on my message yesterday @amsichani @jenniferisasi - please try to consolidate this, #1751, #1738 (and maybe #1525?) into one issue/PR pair, with the most up-to-date ideas and decisions you two have made. I'm concerned that all these tasks seem to have halted forward progress. Please contact me if you need any direct help!

@amsichani
Copy link
Contributor Author

thanks for these comments @mdlincoln . I will now close this as the information and discussion is now on #1767 and will eventually open a new PR(s).

@amsichani amsichani closed this May 6, 2020
@mdlincoln mdlincoln deleted the issue-1738 branch June 3, 2020 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants