Add French translation #213

huey735 · 2019-08-23T17:43:20Z

Building on top of #204

I corrected the logic of liquid flag for the outdated translation alert tool and removed the 'version' variable from the front matter of the download files in order to in agreement with #206

m52go · 2019-08-26T14:21:50Z

@huey735 the French translation looks good. But I can't merge this PR for the issue noted below.

The outdated translations alert tool still doesn't seem right to me. In my testing, it appears on any non-English page no matter what. If I understand your intentions correctly, what you probably want instead are flags for each language (implemented as page variables) that can be toggled every time an update is made to the corresponding English page. Then the outdated-translation text would show on every non-English page until it's updated with the translation, and the flag variable can then be turned off again.

If you want, I can implement this for you.

FYI in the future, it might be best to make separate PRs for separate objectives so at least the good commits can be merged.

huey735 · 2019-08-26T14:33:06Z

@m52go it looks like this right now

      <!-- Outdated translation tag-->
      {% if page.lang == "de" or page.lang= "es" or "pt-PT" or page.lang == "zh-CN" %}
        <span class="navbar-text" style="color: orange;">Outdated translation</span>
      {% endif %}
    </div>

My idea is to add or remove "page.lang == langcode" depending on what is what's not up to date. Maybe there's a better way to do it.
I think you want to make it more specific to a page? Like if only the vision version of the Portuguese page is outdated, to only show it only on that page?
Maybe it's best to go ahead and remove it for now.

I'm still learning Github. I thought that in order to do two concurrent but separate PRs I needed to work on two different branches but that doesn't seem like the case. I guess I need to work on different forks. I'll improve next time.

m52go · 2019-08-26T14:41:19Z

My idea is to add or remove "page.lang == langcode" depending on what is what's not up to date.

Ah, I see now. Generally it's bad practice to edit code to achieve that kind of functionality. Rather, it's better to structure the code so that changes can be made in the front end (in this case, through Liquid page variables).

Another benefit of this approach: by using page variables, you could mark specific pages as outdated. Otherwise, marking a language as outdated makes it outdated through the whole site, even if it's just one page that needs updates.

I thought that in order to do two concurrent but separate PRs I needed to work on two different branches but that doesn't seem like the case. I guess I need to work on different forks.

No worries. You can make as many branches as you want :)

m52go · 2019-08-26T14:45:42Z

So, concretely, I would do something like this.

Make an outdated_translation tag in the front matter of each page. Set it to false.

Then make the condition look like this:

{% if page.outdated_translation %}
    <span class="navbar-text" style="color: orange;">Outdated translation</span>
{% endif %}

Then whenever an update is made to an English page, all the non-English pages should have their outdated tags set to true, so the user sees the outdated message. Then, as the translations are updated, the flags are set back to false, and the message goes away.

huey735 · 2019-08-26T14:49:27Z

No worries. You can make as many branches as you want :)

Yeah but this 'addfrench' branch of mine also inherited the commits of the 'updatetranslation' branch from #204. I need to figure out how to keep them separate.

m52go · 2019-08-26T14:56:57Z

Oh I see. You could git checkout master and then make a new branch from there.

@m52go

On @m52go's advice I changed the construction of the outdated translation alert tool to be spefic to the page instead of the language. I added a "outdated" variable to the front matter of the relevant translated pages. They'll have the values 'true' or 'false' according to the state.

huey735 · 2019-08-26T15:03:02Z

Updated.
I guess I can close #204, right?

wiz · 2019-08-26T15:08:21Z

_includes/main_nav_tr.html

@@ -4,11 +4,11 @@
      <a class="navbar-brand" href="{{ site.url }}/{{ page.lang }}/"><img src="{{ site.url }}/images/bisq-logo.svg" height="30"/></a>

      <!-- Outdated translation tag-->
-      {% if page.lang == page.lang == "de" or page.lang="es" or "pt-PT" or page.lang == "zh-CN" %}
+      {% if page.lang == "de" or page.lang= "es" or "pt-PT" or page.lang == "zh-CN" %}


did you test this?

That should have been updated with the latest commit as it's different now.
https://github.com/huey735/bisq-website/blob/addfrench/_includes/main_nav_tr.html

m52go · 2019-08-26T15:10:54Z

@huey735 looks like some pages don't have the variable? For example pt-pt/index and pt-pt/vision? I didn't check thoroughly, but those stood out. There may be others.

If it isn't too much work, could you make the variable outdated_translation instead? Will be clearer for future contributors to understand what it means. Sorry. I updated my comment above but I guess it was too late.

huey735

@wiz _includes/main_nav_tr.html has been properly updated. For some reason it didn't show in the previous commit.

m52go · 2019-08-26T15:33:13Z

_includes/main_nav_tr.html

+      <a class="navbar-brand" href="{{ site.url }}/{{ page.lang }}/"><img src="{{ site.url }}/images/bisq-logo.svg" height="30"/></a>
+
+      <!-- Outdated translation alert tool tag-->
+      {% if page.outdated %}


wiz did have a point though...you should really test. This variable also needs to be updated...would be an easy catch with a simple test, as it wouldn't work as intended.

Sorry guys. I'll get to it.

huey735

Let's hope this is it. Thanks @wiz and @m52go for checking me.

wiz · 2019-08-26T15:47:23Z

Can you deploy this to a netlify instance so we can see a preview?

m52go · 2019-08-26T15:47:39Z

_data/strings_en.yml

@@ -303,26 +303,26 @@ stats:
  pBSQDistribution: Mainnet BSQ distribution is slated for April 2019.
  pTotalTrades: >
    <p class='stat-label'>Total Trades</p>
-    <p class='stat-figure'>32,318</p>
+    <p class='stat-figure'>34,535</p>
    <p class='stat-note'>Through June 2019</p>


July

Suggested change

<p class='stat-note'>Through June 2019</p>

<p class='stat-note'>Through July 2019</p>

huey735 · 2019-08-26T15:51:07Z

@wiz I'm afraid I don't know how to do that.

m52go · 2019-08-26T15:54:54Z

pt-PT/vision.md

@@ -4,6 +4,7 @@ title: Vision &lsaquo; Bisq - The decentralized Bitcoin exchange
 ref: vision
 lang: pt-PT
 language: Português (PT)
+outdated_translation: true


this is true...intentional?

Yes. I have purposefully not changed them yet as I want it to through Transifex and I wanted to text the alert tool.

wiz · 2019-08-26T16:00:57Z

@huey735 no worries buddy but IMO this PR needs to be rejected - if you want to make a PR to "add french translations" then it should only contain that and not add unrelated new functionality. it kinda looks like you just commited your work tree with random other things in it, which is fine when you do that on your local tree, but you need to separate features when submitting PRs for merging so that we can cleanly review them one at a time.

huey735 · 2019-08-26T16:04:26Z

@m52go @wiz Ok. I'll figure out how to separate the main three updates from this PR into three new PRs.

m52go · 2019-08-26T16:05:35Z

Yeah I think wiz is right here.

https://github.com/bisq-network/bisq/blob/master/CONTRIBUTING.md#contributor-workflow

Note this part: "Pull requests should be focused on a single change."

Typically I tend to be more lax with this on the website repo since it's not the end of the world if there's a mistake (e.g., unlike the software where money can be lost, etc) but this PR is an example of why it's not ideal to bunch up different changes in the same pull request for reviewers.

@huey735 please make separate pull requests for the French translation, translation flag, etc.

huey735 · 2019-08-26T17:12:04Z

I think I got it.
#217
#218
#219

huey735 · 2019-08-26T17:18:29Z

Thank you very much @m52go and @wiz.

huey735 added 4 commits August 8, 2019 16:10

Update stats and download version

edf175c

Add outdated translation alert tool

24b51fc

Correct logic of outdated translation alert tool

5501e91

I corrected the logic of liquid flag for the outdated translation alert tool and removed the 'version' variable from the front matter of the download files in order to in agreement with #206

Add French translation

06e7e24

Change outdated translation alert tool

c8e3807

wiz reviewed Aug 26, 2019

View reviewed changes

Change variable name for outdated translations

eea9985

huey735 commented Aug 26, 2019

View reviewed changes

m52go reviewed Aug 26, 2019

View reviewed changes

Correct _includes/main_nav_tr.html Liquid

aec3b2c

huey735 commented Aug 26, 2019

View reviewed changes

m52go reviewed Aug 26, 2019

View reviewed changes

Correct stats page text

b4c605f

m52go reviewed Aug 26, 2019

View reviewed changes

huey735 closed this Aug 26, 2019

huey735 deleted the addfrench branch September 8, 2019 15:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add French translation #213

Add French translation #213

huey735 commented Aug 23, 2019 •

edited

Loading

m52go commented Aug 26, 2019 •

edited

Loading

huey735 commented Aug 26, 2019 •

edited

Loading

m52go commented Aug 26, 2019 •

edited

Loading

m52go commented Aug 26, 2019 •

edited

Loading

huey735 commented Aug 26, 2019

m52go commented Aug 26, 2019

huey735 commented Aug 26, 2019

wiz Aug 26, 2019

huey735 Aug 26, 2019

m52go commented Aug 26, 2019

huey735 left a comment

m52go Aug 26, 2019 •

edited

Loading

huey735 Aug 26, 2019

huey735 left a comment

wiz commented Aug 26, 2019

m52go Aug 26, 2019

huey735 commented Aug 26, 2019

m52go Aug 26, 2019

huey735 Aug 26, 2019

wiz commented Aug 26, 2019

huey735 commented Aug 26, 2019

m52go commented Aug 26, 2019 •

edited

Loading

huey735 commented Aug 26, 2019

huey735 commented Aug 26, 2019

	<p class='stat-note'>Through June 2019</p>
	<p class='stat-note'>Through July 2019</p>

Add French translation #213

Add French translation #213

Conversation

huey735 commented Aug 23, 2019 • edited Loading

m52go commented Aug 26, 2019 • edited Loading

huey735 commented Aug 26, 2019 • edited Loading

m52go commented Aug 26, 2019 • edited Loading

m52go commented Aug 26, 2019 • edited Loading

huey735 commented Aug 26, 2019

m52go commented Aug 26, 2019

huey735 commented Aug 26, 2019

wiz Aug 26, 2019

Choose a reason for hiding this comment

huey735 Aug 26, 2019

Choose a reason for hiding this comment

m52go commented Aug 26, 2019

huey735 left a comment

Choose a reason for hiding this comment

m52go Aug 26, 2019 • edited Loading

Choose a reason for hiding this comment

huey735 Aug 26, 2019

Choose a reason for hiding this comment

huey735 left a comment

Choose a reason for hiding this comment

wiz commented Aug 26, 2019

m52go Aug 26, 2019

Choose a reason for hiding this comment

huey735 commented Aug 26, 2019

m52go Aug 26, 2019

Choose a reason for hiding this comment

huey735 Aug 26, 2019

Choose a reason for hiding this comment

wiz commented Aug 26, 2019

huey735 commented Aug 26, 2019

m52go commented Aug 26, 2019 • edited Loading

huey735 commented Aug 26, 2019

huey735 commented Aug 26, 2019

huey735 commented Aug 23, 2019 •

edited

Loading

m52go commented Aug 26, 2019 •

edited

Loading

huey735 commented Aug 26, 2019 •

edited

Loading

m52go commented Aug 26, 2019 •

edited

Loading

m52go commented Aug 26, 2019 •

edited

Loading

m52go Aug 26, 2019 •

edited

Loading

m52go commented Aug 26, 2019 •

edited

Loading