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

Support for component-translated attributes #57

Closed
wants to merge 2 commits into from
Closed

Support for component-translated attributes #57

wants to merge 2 commits into from

Conversation

bedag-moo
Copy link

Fixes #56.

@bedag-moo
Copy link
Author

@biesbjerg: It's been a month ... have you seen this PR?

@bedag-moo
Copy link
Author

@biesbjerg: Could you make a decision whether you'll merge this? So far, I haven't published my changes to the NPM registry because I did not want to clutter their repo, but if you remain unresponsive I guess a fork and separate NPM package will be my only option.

@biesbjerg
Copy link
Owner

Hi @bedag-moo,

Sorry about the late reply. I won't be merging this because I personally don't see the value of always extracting attributes with certain names. You should use the ' TranslatePipe' to mark attributes translatable imo. Feel free to fork and publish your own package on NPM if you think it will help other.

@biesbjerg biesbjerg closed this Sep 6, 2017
@bedag-moo
Copy link
Author

Thanks for replying.

My use case is that I have a component with 3 attributes, all of which will be translated as a matter of policy. Since we wrap every form elements in this component, adding 3 translate pipes each time would severely clutter up our source code. (That'll easily be several hundred pipe applications per app, and easily thousands across all our apps). So modifying ngx-translate really is the easier solution for me, and possibly others.

But of course, this is your call to make; I'll see about setting up this fork then.

And BTW: Thanks for the great library, it's a real time saver!

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.

2 participants