Add missing localized views

[hishamco]

This fixed issue #127

[dnfclas]

Hi @hishamco, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

[Eilon]

@ryanbrandenburg assigning to you to review and merge. The PR is a bit old but hopefully still valid. Please confirm before merging.

[hishamco]

@Eilon I'll try to check it too

[Eilon]

@hishamco thanks! And sorry for taking so long to get to this.

[hishamco]

No problem @Eilon, but one more thing I notice there're some PR & issues from long time back with no action taken

[hishamco]

The PR is valid because the English version of the views that I added before are not modified since 9-10 months ago 😄

[Eilon]

@hishamco thanks, we're going through some older PRs so hopefully we'll get through them soon.

[ryanbrandenburg]

@hishamco I don't think this is representative of the use cases for localized views. It appears to me that you've basically copied the existing cshtml files and renamed them to *.fr.cshtml. While that will be served when the culture is french, it's not distinct from the English version, they both use stringLocalizers so you get the same result with and without the views you've added. Instead it might be more demonstrative of the feature to lay the page out differently in french, or possibly to have french strings directly embedded into the cshtml.

[hishamco]

@ryanbrandenburg while we're using IViewLocalizer why we need to use french strings in the cshtml which they are already in the resx files?!!

[ryanbrandenburg]

Simply a suggestion to differentiate localized views from IStringLocalizer's. An example use case might be that french speaking customers repeatedly have a concern that other customers don't, that the developer would like to assuage with an extra text box. So long as the french version is visibly distinct (both in cshtml and resulting html) from the english version, beyond just translation, it should satisfy the use case.

[hishamco]

If I understand correctly, the simple thing to do that we should make the keys in french.
One more thing should we need to add all views in french or couple of them to prove the concept?

[ryanbrandenburg]

@hishamco no, I don't think it's sufficient to change the keys we're localizing based off of. This feature is meant to enable things like laying a page out differently for a language which reads up-down rather than left-right, things which change the actual structure of the html. If we just change the keys which we are localizing between the two versions I think users will find it very confusing.

You are correct however that we needn't do more than one example of this, unless you had a specific feature or use case we want to showcase with a second one, which I can't think of.

[hishamco]

Instead it might be more demonstrative of the feature to lay the page out differently in french, or possibly to have french strings directly embedded into the cshtml.

So what you mean by the last statement that you mentioned before?!!

[ryanbrandenburg]

It was a bad idea on my part to suggest using literal strings, sorry for the confusion it has caused.

[hishamco]

Coz I'm confused too, perhaps I miss understood the issue 😞

[dougbu]

@hishamco #127 is about MVC's IViewLocationExpander feature. That changes the view, not the displayed (localized) strings. So @ryanbrandenburg's comments about not bothering w/ IStringLocalizer changes make sense.

What we're concerned about is the inability to observe that a non-default *.cshtml file was selected. For example Login.fr.cshtml and Login.cshtml should contain some obvious differences -- extra bolding, different styles in general, reordering the fields, …

[hishamco]

Thanks @dougbu for your clarification, I 'll modify the PR ASAP
One more thing is there any scenario that IViewLocationExpander needs to change view not the displaying string in the real world applications?

[dougbu]

any scenario that IViewLocaltionExpander needs to change view not the displaying string in the real world applications?

Feature exists for a reason 😈 Examples include alternate layouts for different locales (say ideographic versus non-ideographic), alternate layouts for different (mobile) clients, …

[hishamco]

Examples include alternate layouts for different locales (say ideographic versus non-ideographic), alternate layouts for different (mobile) clients

Yep IViewLocationExpander can used for different mobile clients layouts, but I didn't realize that the layout may different based on locales, and I forgot that my Arabic language is using RTL which is suitable for this scenario 😄

[Eilon]

BTW LTR vs. RTL text doesn't make layout that much more different or difficult. It often boils down mostly to just setting a CSS style that flips the layout and text direction.

[hishamco]

@Eilon you 're right
setting the page layout to RTL is one thing that let the layout looks different, also we may change the banner's images if it's needed

[ryanbrandenburg]

Any progress on this @hishamco?

[hishamco]

Sorry @ryanbrandenburg for delay, but really I got confused little bit, so that's why I asked @Eilon last time.
Now is language direction enough in this case or shall I do something else?

[Eilon]

Yeah I think the modifications in this PR aren't sufficiently compelling because they don't show why someone would want to use this feature.

This feature is presumably useful if you:

1. Need radically different layout. Probably not super common, but it certainly does happen).
2. Don't use the resource localizer for all content. For example, an About page that has lots of text with HTML formatting - it might be easier to localize the view and not just use a resource string.

Note: The sample in Entropy aren't necessarily about best practices, but rather they just show how to use a feature. So for this case, I think just having some relatively arbitrary differences between the views for different locales, that's fine. So the French view could simply have some extra French words somewhere in it to show that it's different.

[hishamco]

Yep, but using Arabic language or any other RTL language is cool, because the user will notice the change furthermore we can have some words somewhere in it

[Eilon]

That's fine too.

[hishamco]

I will make the change ASAP

[hishamco]

@Eilon that changes is almost ready, but I come up with a strange issue, I'm not sure if it's a bug or limitation, whenever I suffix any partial it ignore the actual translation for specific culture, i.e if I have _LoginPartial.ar-YE.cshtml and current culture is ar-YE the view shows the default culture values which is 'en-US' in my case

/cc @ryanbrandenburg @dougbu

[ryanbrandenburg]

Please file that as a seprate issue.

[hishamco]

Unfortunately this blocked right now by issue aspnet/Mvc#5527

[ryanbrandenburg]

aspnet/Mvc#5527 doesn't have to block this, just don't create a _LoginPartial version for now.

[hishamco]

It's not blocking this completely but if someone see the localized version, he/ she will notice that the part of the layout is not localized, so I want to avoid someone to post an issue regarding this, anyhow I will try to push my updates soon

[hishamco]

@ryanbrandenburg the PR has been updated

[ryanbrandenburg]

@hishamco I think we're having a communication problem here as the cshtml of this PR still seems to be identical to that of the base (unless I've missed a change), only you've changed French to Arabic. The best way I can think of to convey what we want from this issue was to just write the code out, it's #180.

[hishamco]

@ryanbrandenburg what I have seen in the PR #180 is the same English version with extra row, I focused to change the look and feel of the layout, so the users will notice the view changes immediately, my mistake is I missed to change one of the view by adding a bold text like what you did. So either I add the missing in my PR or I will close it

[ryanbrandenburg]

That's my bad @hishamco, I missed the css addition amongset the resx changes, this is a perfect example case for this feature.

However, we have functional tests which rely on the french resources here. We'll need to either return everything to french or have french AND Arabic. Either way you can leave the css changes.

In addition, were there changes to any of the cshtml files other than _Layout? If not please revert the rest of the View Localized cshtmls.

[hishamco]

The current changes in _Layout and the partials, but I will add the change that you made in Home.cshtml, so the user will able to see the changes both in the layout and the view itself, I think @Eilon mentioned this previously when he talked about add bold strings.
Regarding the French views I will bring them back to avoid breaking any tests

[hishamco]

Any updates on this?!!

[ryanbrandenburg]

@hishamco per your next to last comment we were waiting for you to revert the French views to fix our tests.

[hishamco]

@ryanbrandenburg It's my bad, I forgot that I replace the views, I thought that I add Arabic version and delete the French one 😄

[hishamco]

@ryanbrandenburg could you please review the latest updates

[ryanbrandenburg]

This demonstrates the use case really well, but there are also some files added that we don't need, I'm going to take your commit but remove the extras in a second one. Thanks for doing this!

[ryanbrandenburg]

Closed in be24723

[hishamco]

I understand your consideration
thanks