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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(module:datepicker): fix multiple CultureInfo properties #1492

Merged
merged 3 commits into from May 11, 2021
Merged

fix(module:datepicker): fix multiple CultureInfo properties #1492

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dda8709
fix(module:datepicker): fix multiple CultureInfo properties
anranruye May 9, 2021
dc104c2
fix(module:datepicker): fix multiple CultureInfo properties
anranruye May 10, 2021
70cba0d
Merge branch 'master' into FixDatePickerMultipleCultureInfo
anranruye May 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 8 additions & 3 deletions components/date-picker/internal/DatePickerBase.cs
View file
Open in desktop
Expand Up @@ -98,12 +98,17 @@ public DatePickerLocale Locale
[Parameter]
public override CultureInfo CultureInfo
{
get { return base.CultureInfo; }
get
{
return base.CultureInfo;
}
set
{
if (!_isLocaleSetOutside && base.CultureInfo != value && base.CultureInfo.Name != value.Name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am worried about this a bit. What if a consumer decides to use the same CultureInfo as the default but only changes some minor part, like CurrencySymbol? If I understand this correctly, new CultureInfo will not be picked up...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand. To change some minor parts, the user can assign a new Locale instance to the Locale property, which will override the CultrueInfo settings. And, I think this is for DatePicker only, so why do you use CurrencySymbol as the example? And, if the user don't want to override the language of only a single component instance, he want to apply a global configuration, I think this should be done at the application startup stage, not at runtime.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearly my example was not a perfect one, as obviously you are right - currency has nothing to do with date. And because DatePicker anyway will currently use Locale then whatever user chooses will not have any effect.

As I wrote in my previous comment, I think this separation into Locale & CultureInfo may not be the best choice. But I am not the one to decide here. I do think though a consumer of the lib should have ability to set a different locale for each component. But this could be also achieved by scoping services to a component if we are going to do this through DI.

{
_locale = LocaleProvider.GetLocale(value.Name).DatePicker;
}
base.CultureInfo = value;
if (!_isLocaleSetOutside)
_locale = LocaleProvider.GetLocale(base.CultureInfo.Name).DatePicker;
}
}

Expand Down