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 pluralization feature to directive #304

Merged
merged 3 commits into from
Mar 10, 2018

Conversation

GregPeden
Copy link
Contributor

This adds pluralization support to the v-t direction using the additional property "choice". The implementation is slightly sloppy but lightweight in that it makes use of the existing division between $t and $tc. The main reason I wanted this was to take advantage of the performance optimization opportunity afforded by the directive use case.

I have included unit tests but I have not updated the documentation since I am not certain of your workflow structure you're using for docs.

Sorry I committed the dist file before seeing that your contributing guidelines ask that it not be committed, as a tip for the future I suggest you add the dist folder to .gitignore and have GitHub compile the dist folder for you. In this case you'll probably have to merge the PR and then run a recompile and merge that in to capture any other changes since my fork on March 4th.

@GregPeden
Copy link
Contributor Author

btw I think this will fail 3 unit tests however those same three tests failed in the fork I took, ie. those tests are not related to the scope of this PR.

@codecov-io
Copy link

codecov-io commented Mar 5, 2018

Codecov Report

Merging #304 into dev will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #304      +/-   ##
==========================================
+ Coverage   96.11%   96.12%   +0.01%     
==========================================
  Files           9        9              
  Lines         617      620       +3     
==========================================
+ Hits          593      596       +3     
  Misses         24       24
Impacted Files Coverage Δ
src/directive.js 95.12% <100%> (+0.38%) ⬆️

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 acd498f...94bdf4a. Read the comment docs.

Copy link
Owner

@kazupon kazupon left a comment

Choose a reason for hiding this comment

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

Thanks!

Could you revert dist/*.js please?
I build these files when vue-i18n release.

@GregPeden
Copy link
Contributor Author

OK done

@kazupon
Copy link
Owner

kazupon commented Mar 7, 2018

Thanks!
In additionaly, I hope to revert package-lock.json changing. 🙏

@GregPeden
Copy link
Contributor Author

Yeah I did that already

@@ -1,19 +1,9 @@
{
"name": "vue-i18n",
"version": "7.4.2",
Copy link
Owner

Choose a reason for hiding this comment

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

Please revert package-lock.json changing. 🙏

@kazupon kazupon merged commit 8378859 into kazupon:dev Mar 10, 2018
manniL pushed a commit to manniL/vue-i18n that referenced this pull request Mar 10, 2018
… by @SirLamer

* Add "choice" quantity support to v-t directive

* Add unit tests

* Revert dist changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants