Translatable with fallbacks

[pxpm]

WHY

BEFORE - What was wrong? What was happening before this PR?

replaces #4262

Applies the solution me and @tabacitu discussed detailed here: #4100 (comment)

[tabacitu]

Thank you @pxpm . I'll take a look as soon as we're done with the MUSTs in the v5 board. Please take a look at the email I just sent for perspective 💪 Let's do this!!!

[kronos-collector]

Is there any way this fix be extended to the show.blade.php as well? It corrects the functionality on edit but you can still only preview/show the default locale.

[tabacitu]

Appologies for taking a long time to review this Pedro. But I gave it a look on Tuesday and my spidey-sense was triggered. Wanted to sleep on it. After I looked at it today, I think I know what triggered my spidey-sense, let's see if my concerns are valid:

Seems like this makes changes in other operations that UpdateOperation. Namely List and Show. Why? Is that really necessary? From my last comment on it I was under the impression we only needed to change the behaviour on the UpdateOperation, for translatable items. Which would be like 10% of users. The way this is coded now, it would affect 100% of all users. Some translatable methods are called even if they're not using translatable... for all major operations. Yikes! See my other inline comment where I ask if that's necessary to change ListOperation and ShowOperation, let's investigate a bit. Maybe I have my green light and don't remember it 🤷‍♂️

[pxpm]

I tried the video way, it was getting messy (skill issue). Here goes the script:

For all purposes there are 3 available languages in the system: Romanian, English and Portuguese.
The application is configured with locale => en and the fallback_locale => en.
All the products have English translation except 2 (ONLY PORTUGUESE and ONLY ROMANIAN).

ListOperation

Current version

ENGLISH AS SELECTED LANGUAGE: missing strings that are only translated in RO or PT.

PORTUGUESE AS SELECTED LANGUAGE: missing RO only strings

ROMANIAN AS SELECTED LANGUAGE: missing PT only strings

This branch

ENGLISH/PORTUGUESE/ROMANIAN AS SELECTED LANGUAGE: all en, pt and ro strings show.

Disabling useFallbackLocale in config/operations/list bring back the previous behavior.

You pointed a fair question, what happen if you have your panel in RO and you click to edit a non-translated in romanian ?
And what you expect, it's what happens in this branch:

CLICK TO EDIT - PT AND EN: translated in PT and EN but not RO

CLICK TO EDIT - ONLY EN: translated only in EN

UpdateOperation

Adding to the images showed above, when you select any language to "translate the entry from", the translation notice disappear and the inputs are filled with the selected locale:

ShowOperation

Current version

*ENGLISH AS SELECTED LANGUAGE: the entry is only translated in RO or PT:

This branch

*ENGLISH AS SELECTED LANGUAGE: the entry is only translated in RO or PT:

Similar to List, disabling the useFallbackLocale bring back the old behavior.

[tabacitu]

My understanding the bug was in the fact that we didn't set locales on models. (in any operation).

I've just gone through the bug report. To me it does NOT look like anybody has requested a fallbackLocale. The reported problem was that we pre-populated the fields with the fallback. At least that's how I read it... So it looks like this PR does a lot more than what was requested.

---

Excellent screenshots, thank you Pedro 🙏 I have a much clearer picture of how it all looks and works now, thank you.

---

The bubble/notification needs a little beautifying here:

• probably a good idea to make it info / blue? would look good on all themes;
• probably a good idea to turn the button inside the bubble into a btn-link or something (still with a dropdown); it's confusing to have two buttons to change the language, one next to the other (one inside the bubble, one outside it);

---

I don't remember planning/agreeing to create this useFallbackLocale that spiders into every operation... but then again you know me, even if we did talk about it, I wouldn't remember it 😅

I don't think having useFallbackLocale = true is a good idea. I think it makes the whole translatable experience a lot more confusing. I wouldn't recommend anybody use useFallbackLocale, and I would not agree to make it the default in the next version either. So then the question becomes... why do we have it in the first place?

1. Do you remember @pxpm ? How did this idea come up? Why do we have this thing about using the fallback locale at the admin-panel/UI level... when Spatie has already provided a way to do this... at the model level? Afaik if Spatie has provided this at the model-level, if people DO want this fallback behaviour, but ONLY in their admin panel... we can instruct them to create a AdminableProduct extends Product model for their CRUD, and they do the Spatie config in that model. That would achieve the same result in that very unlikely (in my opinion) scenario where the people want this, without us having to do a lot of code changes, to all our operations.

2. Right now it looks to me like it'd be better to just remove fallbackLocale altogether, with all its tentacles, and keep it simple by doing what we promised to do in [Bug] translatable fields can't be empty on related languages #4100 (comment) . I don't have any problems with what we said we'd do there (except some beautifying). Looks like all the triggers from my spidey sense come from useFallbackLocale.

Help me figure out (1) please @pxpm . Let me know if you still this is worth doing after my objections above. And how this config came to be, I really don't remember.

[pxpm]

So it looks like this PR does a lot more than what was requested.

For me this PR streamlines the whole translation process. Every time we get a model, we set the appropriate locale on it, I really don't care if we called it show, update or list operation. We get a model (In CRUD), and we set the locale on that model.

I don't remember planning/agreeing to create this useFallbackLocale that spiders into every operation...
You didn't, this was my judgment call.

when Spatie has already provided a way to do this... at the model level? Afaik if Spatie has provided this at the model-level, if people DO want this fallback behaviour, but ONLY in their admin panel... we can instruct them to create a AdminableProduct extends Product model for their CRUD

I've proposed this solution on other situations and you never agreed to that solution. and IMO you were right and that's why we had other solution in place to don't recommend this.

Basically our useFallback is an "admin configuration" for the fallbackAny of spatie, without the need for the developer to do any additional configuration in the model.
That's what we do in: https://github.com/Laravel-Backpack/CRUD/pull/4267/files#diff-53590d152d70c4232c2ab7c2c670a7978e146cd6179c7e407854ebe6b224fe27

Plus, the settings Spatie provides, are either:

• Configure in the model the fallback local (for that you need to make sure your fallback locale has the translations).
• Configure the fallbackAny in a service provider for the whole translation process (not per model).

We use the later, so we have 3 layers of translation "per model".

• Locale is translated we show it.
• FallbackLocale is translated we show it.
• Locale and Fallback are not translated we use the fallbackAny. (behind a setting).

I think there is no sense arguing this here.
I will remove all that from here and leave only the update stuff, hopefully we can merge this this week.

################################

Regarding what we can actually change in this PR:

probably a good idea to make it info / blue? would look good on all themes;
Nop, it does not look good on all themes, you can try it just by changing the css classes.

probably a good idea to turn the button inside the bubble into a btn-link or something (still with a dropdown); it's confusing to have two buttons to change the language, one next to the other (one inside the bubble, one outside it);

I personally I don't link the link, would have preferred the buttons, but 🤷

[tabacitu]

Yes, I'm happy with the implementation now. Thank you Pedro. Let's do a first round of polish please! I see quite a few things here that could use a second look.

[pxpm]

This is the current implementation:

Added a setting that allow developer to disable this behavior called: showTranslationNotice that defaults to true.

Please evaluate and let me know.

Cheers

[tabacitu]

Gave it another look. Love it. Good to go imho!