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

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion components/core/Base/AntInputComponentBase.cs
View file
Open in desktop
Expand Up @@ -94,7 +94,7 @@ public virtual TValue Value
public string Size { get; set; } = AntSizeLDSType.Default;

[Parameter]
public CultureInfo CultureInfo { get; set; } = CultureInfo.CurrentCulture;
public virtual CultureInfo CultureInfo { get; set; } = CultureInfo.CurrentCulture;
Copy link
Member

Choose a reason for hiding this comment

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

I think I made a mistake here and we should actually use LocaleProvider.CurrentLocale.CurrentCulture.

Copy link
Member Author

@anranruye anranruye May 10, 2021 •
edited

Choose a reason for hiding this comment

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

My view is just the opposite. LocaleProvider.CurrentLocale.CurrentCulture is wrong. LocaleProvider.CurrentLocale.CurrentCulture returns the cultureInfo wrote in the json file. If the user set a culture("xx-xx") which has no corresponding json file, then _locale.CurrentCulture will fall back to en, but CultureInfo property is xx-xx.

I mean, actually CultureInfo property may be different from Locale.CurrentCulture. If LocaleProvider.CurrentLocale.CurrentCulture is right, then we should also do somethink like:

        public override CultureInfo CultureInfo
        {
            get
            {
                return base.CultureInfo;
            }
            set
            {
                _locale = LocaleProvider.GetLocale(value.Name).DatePicker;
                base.CultureInfo = _locale.CurrentCulture;
            }
        }

which is unreasonable.

Copy link
Member

Choose a reason for hiding this comment

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

I think the problem we have here is that Locale is trying to do the same thing as CultureInfo plus some AntDesign specific stuff. I agree it is still needed, but possibly could be hidden behind CultureInfo and not really exposed to the user. Maybe as an option, through DI, for advanced scenarios. This way CultureInfo could be used as the only definition of culture/locale. Appropriate json would be retrieved based on CultureInfo to cover the AntDesign specific requirements. This way we could skip the double-data entry. Or allow json to "override" CultureInfo if necessary. I think there was an idea to "redo" the whole localization logic, but no one decided to tackle this problem, and this is indeed a problem as it will affect the whole library. @ElderJames what do you think?


/// <summary>
/// Gets the associated <see cref="EditContext"/>.
Expand Down
14 changes: 5 additions & 9 deletions components/date-picker/internal/DatePickerBase.cs
View file
Open in desktop
Expand Up @@ -96,13 +96,14 @@ public DatePickerLocale Locale
}

[Parameter]
public CultureInfo CultureInfo
public override CultureInfo CultureInfo
{
get { return _cultureInfo; }
get { return base.CultureInfo; }
set
{
_cultureInfo = value;
_isCultureSetOutside = true;
base.CultureInfo = value;
if (!_isLocaleSetOutside)
_locale = LocaleProvider.GetLocale(base.CultureInfo.Name).DatePicker;
}
}

Expand Down Expand Up @@ -240,9 +241,7 @@ protected DatePickerStatus[] _pickerStatus
protected bool _isClose = true;
protected bool _needRefresh;
protected bool _duringManualInput;
private bool _isCultureSetOutside;
private bool _isLocaleSetOutside;
private CultureInfo _cultureInfo = LocaleProvider.CurrentLocale.CurrentCulture;
private DatePickerLocale _locale = LocaleProvider.CurrentLocale.DatePicker;
protected bool _openingOverlay;

Expand Down Expand Up @@ -405,9 +404,6 @@ protected virtual async Task OnBlur(int index)

protected void InitPicker(string picker)
{
if (_isCultureSetOutside && !_isLocaleSetOutside)
Locale = LocaleProvider.GetLocale(_cultureInfo.Name).DatePicker;

if (string.IsNullOrEmpty(_pickerStatus[0]._initPicker))
{
_pickerStatus[0]._initPicker = picker;
Expand Down