projectfluent / fluent.js

JavaScript implementation of Project Fluent
https://projectfluent.org/
Apache License 2.0
937 stars 77 forks source link

Tutorial/API Docs issues #120

Open brettz9 opened 6 years ago

brettz9 commented 6 years ago

From my review of the tutorial and API docs:

  1. It looks like http://projectfluent.org/fluent.js/fluent/ may need to be updated as it's referring to http://projectfluent.io/ which is giving a 404
  2. I see on MDN one argument missing from http://projectfluent.org/fluent/guide/functions.html#datetime : hourCycle
  3. Under the tutorial docs for Number, I am not clear on the meaning of this one sentence: "The second example may be used to pass additional formatting options to the NUMBER formatter for the purpose of choosing the correct plural category".
  4. For https://github.com/projectfluent/fluent/wiki/Fluent-and-ICU-MessageFormat , I think you want the examples with Fluent.NumberArgument to instead use Intl.MessageNumberArgument
  5. Coming through https://github.com/projectfluent/fluent.js/tree/master/fluent-dom , one doesn't get automated documentation of DOMLocalization or Localization such as is within https://github.com/l20n/l20n.js/blob/master/docs/dom_localization.md and https://github.com/l20n/l20n.js/blob/master/docs/localization.md -- which would be nice to have available from fluent-dom.
  6. I think it could help to have some clarification about the differences between the seemingly overlapping projects (and overlapping APIs), fluent-web (which isn't referenced on the main README), fluent-dom, and the separate l20n.js project (which incidentally sees like it should be dependent on fluent-dom but isn't).
  7. I think the above-referenced API documentation could also probably benefit from documenting the available built-in classes (MessageArgument, MessageNumberArgument, and MessageDateTimeArgument) for use as part of partial arguments within format (if not the other exports, mapContextSync and CachedIterable) since looking through the API docs alone didn't make this apparent. If l20n.js' use of List (apparently Intl.ListFormat) as referenced at http://l20n.org/learn/builtins is also available, it would be nice to have this documented.
  8. If l20n.js' use of | for message line breaks is also available in fluent.js, it is not documented as such at http://projectfluent.org/fluent/guide/text.html (as it is at http://l20n.org/learn/working-with-text-multiline-interpolation ). (And if it isn't, I think it should be drawn out as a special feature of l10n)
  9. If l20n.js' use of a built-in LEN as documented http://l20n.org/learn/advanced-selectors is also available in fluent.js, it is not documented at http://projectfluent.org/fluent/guide/functions.html#built-in-functions (and if it isn't, I think it should be drawn out as a special feature of l10n). Also TAKE is mentioned in the example code at http://l20n.org/learn/complex-example without detailed explanation there, and it is not clear whether it is available by default through l20n.js or through fluent.js. (Moreover, http://l20n.org/learn/complex-example is surfacing a TypeError on the page but as there is no issue tracker for https://github.com/l20n/l20n.js I can't report it there.)
  10. In the example at https://github.com/projectfluent/fluent/wiki/Get-Started#high-level-api, the second new Localization should instead be new DOMLocalization and the first is missing the required second argument.
  11. I'd think another use of Terms that might be called out is use of aliases for representing repeated Unicode (as XML entities did). For example, I'd presume one could define -ellipsis, -copyright, etc. for these symbols if repeating their use, with the use of Terms here designating they were just local definitions perhaps set up by the translator for their own convenience.
brettz9 commented 6 years ago

While I understand it may take a little time to investigate these issues, I'd be most appreciative for a clarification now about the differences between the seemingly overlapping projects (and overlapping APIs), fluent-web (which isn't referenced on the main README), fluent-dom, and the separate l20n.js project (which incidentally sees like it should be dependent on fluent-dom but isn't). Is fluent-web meant to take over the role of l20n.js? I'd rather not be digging into the code of, or making PRs for, a project you're deprecating...

stasm commented 6 years ago

@brettz9 Thank you so much for the thorough review of the documentation! There sure is a lot of room for improvement :) I'll be working this week and next week on addressing the issues you've found.

To answer the questions from your second comment: yes, fluent-web is meant as the replacement for the now-deprecated l20n.js. I opened https://github.com/l20n/spec/issues/17 to properly mark l20n.js as deprecated and I'll update Fluent's READMEs (including fluent-web) accordingly.

brettz9 commented 6 years ago

No worries--these were mostly small recommendations, and I am quite eager to see the web community (including myself) benefit from what appears to me to be such a very well thought out, experience-driven and comprehensive framework.... Thank you all for your hard work!

brettz9 commented 6 years ago

Any chance of reviewing these?

zbraniecki commented 6 years ago

Assigning stas since he started reviewing it already. :)

anlexN commented 6 years ago

who can tell me fluentjs tutorial? i don't understanding how to use fluent in my web project! seemingly, fluentjs is javascript specification, like ecmascript, not a framework.

brettz9 commented 6 years ago

@anlexN : No, there is a framework for it though it defines a file format . For the framework, see especially https://github.com/projectfluent/fluent.js/tree/master/fluent . For the format of the file, see https://projectfluent.org/fluent/guide/ and https://projectfluent.org/play/

@stasm : Any chance of a review? Also, FYI, I've added a new item at no. 11 suggesting Terms mention their possible use for Unicode (like how XML entities can be used).