Skip to content
This repository has been archived by the owner on Nov 21, 2018. It is now read-only.

#264 build process - adds various i18n helpers, html-based templates #276

Merged
merged 19 commits into from
Mar 17, 2015

Conversation

snostorm
Copy link
Contributor

@snostorm snostorm commented Mar 5, 2015

See #264 for background on the larger work happening.

  • Adds support for {{link "i18n_links_obj_reference" }}, or {{link "string_static" link.dynamic.from.scope }} type helpers
  • Adds {{hb 'i18n_reference'}} for allowing complex, Handlebar friendly-strings to be defined in localization templates.json files.
  • Adds {{i18n "key.string"}} {{i18n "scope" dynamic_key_portion}} type helpers

For use-cases on these, look at the en localization's home.html and template.json. The cn localization acts as an example of consuming the defaults en provides.

To do: commenting, documentation. Making the template.js task more "DRY".

Note: eventually, when this is merged, we will not have duplicate copies of the home.html in each localization (and index.md will be retired.)

@therebelrobot
Copy link
Contributor

Yeah, I'd say lets not merge this for now because of the proof of concepts in there. Is there any reason why that html template needs to be in the content/ folder and not the source/templates/ folder? I can't see any difference between the cn and the en html templates.

@snostorm
Copy link
Contributor Author

snostorm commented Mar 5, 2015

@therebelrobot they're currently in the "content" directory to allow reusing the same task workflow we already have in place. I'll need to tweak things more to make them work from a different source (especially the one file to many i18n outputs this change will imply.)

Mostly wanted to get this up so we can ping a few i18n groups for their thoughts. No rush to merge.

@snostorm
Copy link
Contributor Author

snostorm commented Mar 5, 2015

At this point we're looking for feedback / suggestions on the patterns used in the sample home.html within that pull request. Some files (like the homepage) would move to sharing a single template like this with i18n hooks where articles (like FAQ, ES6) would remain markdown.

"slogan": "将 {{link 'pages.es6'}} 带入 Node 社区!"
},
"links": {
"pages": {

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@therebelrobot
Copy link
Contributor

@therebelrobot they're currently in the "content" directory to allow reusing the same task workflow we already have in place. I'll need to tweak things more to make them work from a different source (especially the one file to many i18n outputs this change will imply.)

IMO it's easier to maintain structure as the website group if the HTML is only in one location, hence the source/templates folder. Is that where you eventually want it to reside? or are we moving toward putting html for good in the content folders? Because I would strongly -1 the latter.

@snostorm
Copy link
Contributor Author

snostorm commented Mar 7, 2015

@therebelrobot the goal is to move the individual (complex) HTML-based pages out, probably to source. Right now it is just easier (for developing other aspects) to re-use the build flow of content. I hope to transition soon.

But, we might still want to allow for .html articles the in content directory for some other cases, although we'd generally discourage against it unless there was a good case?

@therebelrobot
Copy link
Contributor

Leaving it there for development is fine. I'm only hesitant of it for production because of separation of concerns (we'd be handing off structure and styling to the 18n groups, when that's kinda our job here). If we want the i18n groups to be able to add language-specific pages, and link to them either in their markdown or elsewhere, that might work, but those pages wouldn't be accounted for in the navigation. I'm less worried about that, but anything that needs to be replicated in each locale, IMO, should be templated in production.

@snostorm
Copy link
Contributor Author

snostorm commented Mar 7, 2015

I still agree re: a 100% template approach for anything re-used. This "home.html" proof-of-conept won't hit production (until it is moved out.)

The .html option in the content directory really isn't that far off of what we have now Markdown (in that our Markdown rules allow for inline HTML anyway.) I think we just make it clear that (if the option stays) it is really only an alternative option for basic article content only? It might be nice for some i18n groups where their language + Markdown may be harder to manage, say for right-to-left use cases. This just lets the auto-magic of Markdown to be turned off. Anyway, an easy enough change to make later one way or the other.

@snostorm
Copy link
Contributor Author

snostorm commented Mar 8, 2015

Alright ... I basically have the home.html working from source/templates locally. It uses a combination of a source/content.json manifest and a source/languages.json to ensure specific pages are generated for every localization.

I'll use this later to help to i18n fallback to English as well (when articles like ES6 aren't yet translated we can still show them, but with (localized) messaging asking for help translating and with the localized nav menu, etc. still surrounding the article. Plus we can rel=canonical to make sure search engines pick up the right version.

@snostorm
Copy link
Contributor Author

FYI this now includes the new content task which I discussed a few days ago. The watch still needs to be configured to auto-regenerate content when certain things change.


.home-download
background: #eee
display: flex

This comment was marked as off-topic.

@Fishrock123
Copy link
Contributor

Seems mostly good to me, though I haven't tried running it.

Only critique I have on concept is that it does make it more difficult for people who only speak non-english to understand what line goes where. The json should be roughly ordered appropriately is possible. Unfortunately, sometimes things have to give in order to make things work well, so that's the trade it looks like we have to do.

@snostorm
Copy link
Contributor Author

@Fishrock123 I think switching the template.json to a dictionary.js (or a combination of a few dictionary files eventually) may be a good option.

Again, this enables inline commenting, which would help i18n teams leave clues for themselves (as they often do now with <!-- ---> in the markdown itself.)

Plus, we can also inline document our own (English/default) dictionary file the same way to also help people follow it.

Thanks for checking it out :)

@snostorm
Copy link
Contributor Author

@Fishrock123 I'd be curious to see you test the branch locally (in addition to making any code changes you're interested in) so we can get this locked in to keep building off of it. Trying not to get to a mega PR, but just to a point where:

a) everything still builds for people
b) we can give direction on needed changes, like the dictionary definitions needed to retire the index.md files.

@Fishrock123
Copy link
Contributor

LGTM if it works, I haven't tried it yet. Is there any way this could be squashed? 16 commits is a lot to parse together..

Otherwise I'll try it soon and do that part myself.

@snostorm
Copy link
Contributor Author

This project already has a fairly noisy merge/commit history that I'm not too concerned. I'd say the vast majority of the commits were fairly intentional and atomic commit history points to show the progress of changes. Squashes tend to lose the step-by-step if you need follow the flow in reverse.

A few of the odd style ones add a bit of noise, but not a ton in the context of a larger branch.

Re: testing locally, I'm willing to wait for that ;) I'd rather you see it work for yourself.

@Fishrock123
Copy link
Contributor

Oops, weekend happened. I'll try to remember to do a test and finalize of this in the morning.

@snostorm
Copy link
Contributor Author

Thanks @Fishrock123. Starting to queue up more in #287 ;)

@Fishrock123
Copy link
Contributor

@snostorm please review the last commit. LGTM to land so long as our Chinese group is ok with switching their page, I guess?

@snostorm
Copy link
Contributor Author

@Fishrock123 code changes lgtm. The Chinese languages team group approved a while ago on nodejs/nodejs-zh-CN#128 ;)

Just running now. Will merge if that works (as I assume it will.)

snostorm added a commit that referenced this pull request Mar 17, 2015
#264 build process - adds various i18n helpers, html-based templates
@snostorm snostorm merged commit 88a5c8d into master Mar 17, 2015
@snostorm
Copy link
Contributor Author

Correction: the /content/cn/index.md still exists on this PR so it'll win over the default template (no change)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants