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

Add Locki to API documentation #8798

Closed
mikeal opened this issue Sep 27, 2016 · 39 comments
Closed

Add Locki to API documentation #8798

mikeal opened this issue Sep 27, 2016 · 39 comments
Labels
doc Issues and PRs related to the documentations.

Comments

@mikeal
Copy link
Contributor

mikeal commented Sep 27, 2016

We do no currently have localizations for API documentation.

The system we use for the regular website is pretty heavy, doesn't port very well, and isn't very tolerant of regular changes.

Locki is a new service for creating a localization community and managing the translations.

Here is their URL: https://locki.io

/cc @chikathreesix who runs the service and can answer any questions or concerns people have.

@mscdex mscdex added the doc Issues and PRs related to the documentations. label Sep 27, 2016
@yosuke-furukawa
Copy link
Member

That is pretty good. we are discussing about how to localize our API documentations. nodejs/docs#11 (comment)

CC: @nodejs/nodejs-ja

@chikathreesix
Copy link

Thank you @mikeal for posting this issue and thanks @yosuke-furukawa for your support!

Locki is a service that allows your website's visitor to translate your content right on your website by inserting a few lines of JS. Now you can also form your translation team like github to enable only your team to edit the translations so I believe this is really beneficial for OSS community like Node.js to translate its documentations.

Please try Locki by registering the API doc website to it and inserting the JS into your HTML.

If you have any questions, feel free to ask me anything. We have just launched so it might not have enough features but we are willing to build the features that the community want so give us any feedbacks!

@watilde
Copy link
Member

watilde commented Sep 27, 2016

This looks like what we wanted! In our use case of translation for the API docs, I guess we need to create a project on the locki for every single page of the docs since the segment has only the words which are used in the page we set, right? @chikathreesix

e.g.
https://locki.io/projects/22/segments

@chikathreesix
Copy link

Thanks for trying this out @watilde!
You don't need to register every single web pages. Once you register the root URL and insert JS to pages that belong to the root page and also you want to translate, Locki automatically add pages and also check updates when the pages get accessed.
So I guess in this case, you guys can register https://nodejs.org/api/ since nodejs.org has translated versions already.

@jasnell
Copy link
Member

jasnell commented Sep 27, 2016

Key question is where the translations live. Are they hosted i Locki's content store? If so, what are the service guarantees so that those things don't disappear suddenly (not expecting that, of course, but just to be safe...)

@chikathreesix
Copy link

Hi @jasnell, currently we are hosting the data on the Postgres db on Heroku and fetching them from the API but will be switching to S3 in few weeks to directly host JSON files for the translations so that the translations will never disappear unless S3 is gone. In this case, you guys can also download the JSON files directly from S3 anytime as you like.

@jasnell
Copy link
Member

jasnell commented Sep 27, 2016

@chikathreesix ... first off, great to meet you in person last night! Thank you for the clarification. What I'm mostly wanting to make sure is that the translations are backed up so this is great. I appreciate it. The other question I have is whether or not the translations could be version controlled in any way. Specifically, when a section of the documentation is modified, how are the changes those those individual sections handled? Would it continue to show the old translation or would there be some indication that something has changed and that an updated translation is required?

@mikeal
Copy link
Contributor Author

mikeal commented Sep 27, 2016

As long as there is an export API we can pull down that export every day "just in case."

@chikathreesix
Copy link

Oh James, I am sorry I didn't recognize you from your comment. I am also really glad to meet you in person last night!

When we fetch a HTML, we parse them into something called "segments" which is basically equivalent to TextNode and translations belong to each segments. Whenever Locki finds a pages is updated, it creates a new segment for a updated TextNode which doesn't have any translations yet so translations will never be outdated, however, you need to translate every time it gets modified.
We are planning to implement some kind of notification to the team when it's updated and also show what was the previous TextNode at the same element as a reference.

In terms of a version control, we also save all the translation activities (like commits) to see who did what and also eventually to implement a feature to rollback to a specific point.
https://locki.io/projects/2/activities

@chikathreesix
Copy link

Hi all, I have integrated Locki here as an experiment. Try it there and give me feedbacks. Also let me know if you want to add more languages.
https://locki-nodejs-test.herokuapp.com/api/index.html

In order to edit, you need to create an account either from the module on the website or our platform https://locki.io

Although Locki automatically adds a page when getting accessed, it might take a few seconds to parse it so you might want to refresh the page if it doesn't show the boxes to edit.

@ChALkeR
Copy link
Member

ChALkeR commented Oct 2, 2016

Will this affect only the API documentation (i.e. the /api/ dir)?

@chikathreesix
Copy link

@ChALkeR yup, you can do that so and I think that should the way in this case.

@ChALkeR
Copy link
Member

ChALkeR commented Oct 2, 2016

@chikathreesix, is https://locki-nodejs-test.herokuapp.com/api/index.html the same as what it is actually going to look now? Some feedback then:

  1. «Sign in with GitHub» is broken, and that's probably the option that would be most popular otherwise.

  2. «Sign in with Facebook» is also broken, this leaves only one option available — manually registering on Locki website through email.

  3. Switching between pages on the left resets the language to English.

  4. The slideout animation is strange and inconvenient — I open the page, language selector gets loaded and shown after about 3 seconds. I see «English». I expect that I am able to change the language if I press that, I move mouse over «English», press it, but by that time it slides out on hover, running away from the mouse, so I miss the button I want to click and accidentally click on the link to the Locki website instead, which isn't what I wanted to do or what I expected to happen.

    That happened several times to me while I tried to get this working.

  5. Some edit boxes don't look right on our documentation, see attached image — the box somewhy is zero-height and became a line. And users are supposed to click that box to be able to edit the text, as I understand things.

    spectacle s24248

Those are all rather minor issues, but they will constantly get in the way of users who want to interact with that new feature. Note: I did not take a look at the actual translation tool yet, mostly because GitHub/Facebook auth does not work atm.

@ChALkeR
Copy link
Member

ChALkeR commented Oct 2, 2016

One more issue just noticed: editor on https://locki.io/ (which doesn't actually allow to edit anything, but that's probably because I can't log in) sometimes just redirects to https://locki.io/undefined, which probably means that logic is broken somewhere. To reproduce:

  1. Open https://locki.io/signin.
  2. Switch Language, press «edit». All text snippets on the page gain borders, which probably signifies that they are editable.
  3. Click on Email, Password, or or — it doesn't allow to edit, nothing happens.
  4. Double-click on Email, Password, or or — you will get redirected to https://locki.io/undefined.

@ChALkeR
Copy link
Member

ChALkeR commented Oct 2, 2016

Also, weird things happen sometimes (pretty often, but I didn't catch a correlation to what I click yet):

«Edit» button in English language:
spectacle y24248

Language selection is there, but doesn't work at all:
spectacle m24248

Note that I got uBlock disabled for this test and no other plugins installed.

Overall, this looks alpha quality and untested. -1 from me in the current state, but it might change once this stabilizes and starts working as intended. (Given that we would be able to receive a copy of all translations in case if something goes wrong).

Also I have some security concerns, but I will have time to take a look at things from that perspective only once and if this starts functioning correctly.

@ChALkeR
Copy link
Member

ChALkeR commented Oct 2, 2016

Another blocker from me (and a separate reason for -1 apart from the usability issues above) would be the fact that this is hosted on the same domain — that can have security implications for the whole website in case if Locki becomes compromised (which could happen).

Can we please move the documentation (and everything what we are going to apply this to) to a subdomain (e.g. https://api.nodejs.org/) and place a redirect on https://nodejs.org/api?

@chikathreesix
Copy link

@ChALkeR Thank you so much for your detailed feedback!
We will solve all the issues ASAP.

1. 2. Sign up with github/facebook
I couldn't reproduce this so I'd like to know more details on this issue.

  • What's your browser, its version and OS?
  • How didn't this work? Clicking the button didn't do anything at all?
  • A popup is expected to open, do you somehow completely block it?

3. Language setting over pages
I agree that it should keep the language setting, will fix it ASAP.

4. Animation
Yes I also realized that this is kind of annoying... Let me think about what kind of behavior is the best.

5. Weird box definition
Yes I have recognized this too and looking into what the cause is.

6. /undefined redirect issue
This is really weird... will look into it.

7. Selection doesn't work
Can you see any error happening on the console?

@targos
Copy link
Member

targos commented Oct 3, 2016

@chikathreesix I was able to login with GitHub. Now I have two questions:

  1. How can I add a new language ?
  2. Is the translation stored in such a way that it can be reapplied (on text that wasn't changed in English) when the documentation is republished along with a new release ?

@ChALkeR
Copy link
Member

ChALkeR commented Oct 3, 2016

@chikathreesix

    1. Sign up with github/facebook

Chromium 53, Arch Linux current.

A popup has opened, I successfully authorized the application both on GitHub and of Facebook, but both of those give the same error: * Username may only contain lowercase letters, numbers and single hyphens, and cannot begin or end with a hyphen. I didn't type a username anywhere, so I'm now guessing that it tries to somehow create one from GitHub/Facebook data and fails at that. Note that my Facebook username is chalkerx and it's latin and lowercased. My GitHub username is ChALkeR, but lowercasing it shouldn't be an issue. So I assumed that it just doesn't work, but I hope that this feedback would help to track down the issue =).

6. /undefined redirect issue

This is really weird... will look into it.

Well, it has an event listener which looks like this:

function(e) {
  var t = e.currentTarget;
  "_blank" == t.target ? window.open(t.href) : window.location.href = t.href
}

Given that it's not a link and doesn't have a href property, it's pretty obvious what happens there =). Not sure why it has that event listener attached, though.

Upd:

t.tagName.match(/a/i) && t.addEventListener("dblclick", e._onDblClickLink)
> 'span'.match(/a/i)
[ 'a', index: 2, input: 'span' ]

7. Selection doesn't work

Can you see any error happening on the console?

No, nothing is printed to the console. I just clicked several links, and I can't find the exact pattern. It happens pretty often, though.

@chikathreesix
Copy link

Hi @targos,

  1. Currently only admins can add a language from our platform site but we will enable front end module to create a language as well.
    By the way, I have added you to the team as admin so you can add a language from here.
    https://locki.io/projects/23
  2. Yes exactly. Basically the translations take its original copy as a key so even though the site is updated, while a copy stays same, the translations will be automatically applied. If a copy is changed, basically you need to translate the new copy. But because currently even a minor change like adding a comma is considered as a new copy, I am thinking to show the previous translation placed in the same position as a reference.

@chikathreesix
Copy link

@ChALkeR

Thanks, I got the issue. It tries to use your github username as our username too but doesn't match with our validation. Let me fix this.

@ChALkeR
Copy link
Member

ChALkeR commented Oct 3, 2016

@chikathreesix And what about Facebook? It shows the same error, but it's lowercased.

@ChALkeR
Copy link
Member

ChALkeR commented Oct 3, 2016

@chikathreesix

even though the site is updated, while a copy stays same, the translations will be automatically applied

I see that it uses things like /html/body/div/div[2]/div[2]/ul[11]/li/a for addressation. What would happen if I, for example, insert a paragraph B right before an already translated paragraph А? Will the translation still apply to А, given that its contents weren't changed, or will it be lost?

@chikathreesix
Copy link

@ChALkeR
facebook issue should be same cause I assume.

As mentioned in the previous comment, since it takes a original copy as a key, translations are applied to everywhere that has a same copy so the position doesn't matter, the translations won't be lost. This XPath is stored for a reference not for a key.

@chikathreesix
Copy link

@ChALkeR Your github login should work now. I am not sure about facebook login but if it fails I can track the details now so please try that again.

@ChALkeR
Copy link
Member

ChALkeR commented Oct 3, 2016

@chikathreesix I can confirm that GitHub auth works now, but Facebook auth still fails with the same error.

@ChALkeR
Copy link
Member

ChALkeR commented Oct 3, 2016

@mikeal, is the proposal here to have the documentation world-editable or limited to the nodejs org members only?

@mikeal
Copy link
Contributor Author

mikeal commented Oct 3, 2016

@ChALkeR I think the proposal is to allow the localization be editable by the community and the access controls maintained by whatever system Locki has. For most localizations that is probably preferred, most of the localization communities we have in our GitHub org are a subset of a much larger community in that locale.

@chikathreesix
Copy link

@ChALkeR I got the issue with your facebook account and now it should be fixed.

@ChALkeR and @mikeal
Although you can also limit the editing to nodejs org members only first not to be messed up by random people, the idea of Locki is to open up the funnel more specifically for the localization since the bar for getting into the localization community with Locki becomes easier. So I think the localization specific community eventually might be consist of a bit different members from the core nodejs community.

@targos
Copy link
Member

targos commented Oct 4, 2016

@chikathreesix

While playing around with the tool, I found this:
image

There is some HTML that wasn't isolated.

@chikathreesix
Copy link

@ChALkeR oh, since you are using a same email address as github, you can't create a new account with facebook. Let us think what would be the best way to solve this.

@chikathreesix
Copy link

@targos This is an expected behavior since you might need to change positions of anchor, span, strong tags and so on which appears in a paragraph. So it groups inline elements into a segment.
I agree that it should be nicer in terms of displaying them so we are thinking about display the tags in a simplified format or something else not to show the entire tags.

@ChALkeR
Copy link
Member

ChALkeR commented Oct 4, 2016

@chikathreesix

oh, since you are using a same email address as github, you can't create a new account with facebook. Let us think what would be the best way to solve this.

I don't think that this is very important (logging in from two accounts), the good thing is that everyone could log in through their preferred way.

@jbergstroem
Copy link
Member

jbergstroem commented Oct 4, 2016

This bug is kind of moving into a bug report for locki. My impression reading this is that it doesn't seem mature enough to provide a consistent experience for the animation/popup (which for me is a tradeoff). Going to agree with @ChALkeR in that in its current state its a -1.

Also, if we're doing this I'd like to see a similar solution to what we discussed in the analytics issue -- add a flag/env var so its turned off by default should documentation be built/read locally.

@chikathreesix
Copy link

@jbergstroem Thanks for your feedback. What do you mean by a consistent experience for the animation/popup?

@ChALkeR Issue 3 and 6 was already fixed. For issue 5. weird box definition, I found that that the height of the element is defined 0px in its CSS so we can't fix this from our-end.

@yosuke-furukawa
Copy link
Member

I think we need to collect more opinions about this.
And I would like to hear other localization WG voice.
I will open same issue on nodejs.org repos.

@yosuke-furukawa
Copy link
Member

Oops, I have not read nodejs.org issue guideline. according to that, API docs problem should be discussed on nodejs/node repo.

I would like to progress this issue. Someone has downvote, but someone has upvote, we need to gather more opinions to judge how to handle i18n API docs.
CC: @nodejs/localization

@ChALkeR
Copy link
Member

ChALkeR commented Nov 2, 2016

@chikathreesix

For issue 5. weird box definition, I found that that the height of the element is defined 0px in its CSS so we can't fix this from our-end.

Ah, I see, thanks. Then, it should be fixed on our end.
Current rules (doc/api_assets/style.css#L305-L310):

#toc h2 {
    margin-top: 0;
    font-size: 1em;
    line-height: 0;
    margin: 1.5em 0;
}

Not sure why that combination of margins and zero line-height was introduced… It's since #297.

@Trott
Copy link
Member

Trott commented Jul 15, 2017

This issue has been inactive for sufficiently long that it seems like perhaps it should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.

@Trott Trott closed this as completed Jul 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

No branches or pull requests

10 participants