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

Components rely on translate beeing bound to Vue.prototype.t #586

Closed
raimund-schluessler opened this issue Sep 10, 2019 · 4 comments · Fixed by #2229
Closed

Components rely on translate beeing bound to Vue.prototype.t #586

raimund-schluessler opened this issue Sep 10, 2019 · 4 comments · Fixed by #2229
Assignees
Labels
1. to develop Accepted and waiting to be taken care of bug Something isn't working

Comments

@raimund-schluessler
Copy link
Contributor

With nextcloud/tasks#586 and nextcloud/tasks#601 I realised, that the vue-components rely on the app to bind t to Vue.prototype.t (and probably n to be bound to Vue.prototype.n).

First, as stated https://vuejs.org/v2/cookbook/adding-instance-properties.html#The-Importance-of-Scoping-Instance-Properties, instance properties should be scoped (so start with a $).

Second, I didn't find this in the Readme of the components, so this should either be added, or the vue-components should bind t to the correct prototype themself (don't know, whether this is possible).

Changing the instance properties to be scoped would be a breaking change in case the components cannot bind t themself.

@raimund-schluessler raimund-schluessler added the bug Something isn't working label Sep 10, 2019
@korelstar
Copy link
Contributor

Please have a look at this discussion: #439 (comment) (unfortunately I did not found the time to do the PR I talked about).

Would that be a solution?

@skjnldsv
Copy link
Contributor

Yep, it's because we want to remove it #97

@skjnldsv skjnldsv added the 1. to develop Accepted and waiting to be taken care of label Sep 11, 2019
@skjnldsv
Copy link
Contributor

@raimund-schluessler what do you suggest then in the meantime?
Importing nextcloud-l10n from the library here and use it?
With Vue.prototype.$t ?

@raimund-schluessler
Copy link
Contributor Author

What is the plan here? Did I get it correctly that we should use
https://github.com/nextcloud/nextcloud-vue/blob/b46a4cb341bb6b90ce27daa1097ab54d6ab57fa7/src/l10n.js#L42
(or the mixin) now in every component and don't need the global OC.t and the binding to the Vue prototype anymore?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants