conedevelopment / i18n

Push your Laravel translations to the front-end and use them easily with JavaScript.
https://pineco.de/using-laravels-localization-js/
MIT License
129 stars 21 forks source link

fewer steps for js import usage #2

Closed ctf0 closed 6 years ago

ctf0 commented 6 years ago

i believe it would make more sense if importing the i18n already comes bundled with the translations.

as atm we need to do 3 steps

iamgergo commented 6 years ago

@ctf0 yes, i see your point.

The blade directive, should print the whole script tag? And in the config file the can configure the name of the translations object (in case of conflicts).

I see less relevant the multiple translator instances now, so i agree to pass the window.translations by default.

So it would be two steps:

What do you think?

iamgergo commented 6 years ago

@ctf0 or instead of config, we may let the @translator directive to accept an optional parameter, which will be the name of the window property. If we don't pass anything, translations will be the default.

Like: @translations('myTranslations') --> window.myTranslations

iamgergo commented 6 years ago

@ctf0 We published a version with the steps above, you can see the changes in the code!

ctf0 commented 6 years ago

okay, nice work with the new directive but not quite a solution for the js import, i mean if i have to use the directive, why i need the import ? the directive already attach the translation to the window.

that's why i suggested to bundle the whole thing in the first place.

atm the config has no use, as with the new directive that part has already been taken care of.

ctf0 commented 6 years ago

for the bundling, we can add a small logic that saves the $this->getTranslations() result to a file.

i know it will be kinda tricky to wrap our head around it as we are using App::getLocale() to collect the data first.

but i think it would be better and faster if we just collected all of the available locales, this way as mentioned b4 you don't need to remake the translation each time the user switch the locale and harness the power of blade caching

iamgergo commented 6 years ago

@ctf0 The config needs to be present because it contains where do you want to share your translations. By default, we share the variable everywhere but what if they don't want a variable in some specific cases.

I'm not sure what exactly do you mean under bundling. Can you explain a bit better?

I want to keep the JS separated. Importing the JS gives flexibility, what I want to keep. By the way, it's only one import, it's really not a big deal compared to the flexibility it offers.

I think this way of using translations, first of all, better to those who are making an SPA. For traditional web app or a hybrid web app, you can use the Laravel translations. But it's not the case with component-based SPAs.

But if you are using an SPA, you need to load the translations only once, which means performance is not a priority here (we are talking about time what we can't even feel).

Maybe if the user wants, we could offer an option, to cache the translations. What do you think? Also, an artisan command to flush translations.

I think the file-generation is not a good way to use, it could be a hard thing to optimize and support.

ctf0 commented 6 years ago

but what if they don't want a variable in some specific cases.

using the blade directive already solve this, and you already have the option of using a custom window object, so the manual way which require the config is not needed.

for other points, i will make a PR with what am talking about to make it easier to understand what i mean.

ctf0 commented 6 years ago

@iamgergo can u check the pr and tell me what u think ?

iamgergo commented 6 years ago

@ctf0 we had a loooong week, sorry for the late answer.

So we considered the points what your PR offers and here are the changes we will do:

We create a new PR containing the new changes, including come of your PR, hopefully the following days.

By the way, I really appreciate your efforts, I really really thank you for the ideas!