Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

Add tn / translateWithNumber #15

Merged
merged 2 commits into from
Oct 26, 2018
Merged

Add tn / translateWithNumber #15

merged 2 commits into from
Oct 26, 2018

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Oct 26, 2018

🏆 Enhancements

  • Add tn (translateWithNumber) to translation module
  • Remove sprintf-js because jed already comes with sprintf.

@codecov
Copy link

codecov bot commented Oct 26, 2018

Codecov Report

Merging #15 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #15   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           9      9           
  Lines          80     85    +5     
=====================================
+ Hits           80     85    +5
Impacted Files Coverage Δ
packages/superset-ui-translation/src/Translator.js 100% <100%> (ø) ⬆️
...superset-ui-translation/src/TranslatorSingleton.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b3b4c8d...3e0b514. Read the comment docs.

@kristw kristw added the reviewable Ready for review label Oct 26, 2018
expect(translator.translateWithNumber(undefined, undefined)).toBeUndefined();
});
it('returns original text for unknown text', () => {
expect(translator.translateWithNumber('fish', 'fishes', 1)).toEqual('fish');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need another test, but curious how it behaves with 0 since that is the default num. is that singular?

Copy link
Contributor Author

@kristw kristw Oct 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that depends on each languagePack. I saw there are DSL-ish field in the language packs that defines n for singular and plural.

"plural_forms": "nplurals=2; plural=(n != 1)",

Copy link
Contributor

@williaster williaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Nice add

@kristw kristw removed the reviewable Ready for review label Oct 26, 2018
@kristw kristw merged commit b6fb0e2 into master Oct 26, 2018
@kristw kristw deleted the kristw--translation-num branch October 26, 2018 21:50
@kristw kristw added reviewable Ready for review and removed reviewable Ready for review labels Nov 13, 2018
@kristw kristw added the #enhancement New feature or request label Dec 6, 2018
kristw added a commit that referenced this pull request Apr 17, 2020
* feat: add encodeable utilities

* feat: add types back

* refactor: simplify function calls

* refactor: rename generic type

* refactor: more edits

* refactor: remove unused function

* refactor: rename file

* fix: address comments

* fix: add vega back
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
#enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants