Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Fix datetimepickerlocalization #11474

Merged
merged 21 commits into from
Nov 11, 2020
Merged

Fix datetimepickerlocalization #11474

merged 21 commits into from
Nov 11, 2020

Conversation

memu8
Copy link
Contributor

@memu8 memu8 commented Jul 17, 2020

Description of Change

The Format property of the DatePicker and TimePicker creates consistent changes of the date and time displays. Also, the DatePicker format is consistent with the iOS default for specific region and language settings.

Issues Resolved

API Changes

None

Platforms Affected

  • iOS
  • Android
  • UWP

Behavioral/Visual Changes

The DatePicker and TimePicker displays and pickers might change depending on the format property as well as the regional settings.

Before/After Screenshots

Before:
pic1timepicker

After:
pic4timepicker

Testing Procedure

Create DatePicker and TimePickers on Android, iOS, and UWP and play around with different format properties as well as different regional settings to see if the date and time picker displays and picker stay consistent and follow the region's culture default format for date and time respectively

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@dnfadmin
Copy link

dnfadmin commented Jul 17, 2020

CLA assistant check
All CLA requirements met.

Copy link
Contributor

@hartez hartez left a comment

Choose a reason for hiding this comment

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

First-pass review, looks like we might have some extra changes in here, plus I think we can slim down the UI test code a bit.

@@ -0,0 +1,9 @@
using System.Globalization;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change the file name to match the interface name?

@@ -71,7 +71,7 @@
</Target>

<Target Name="_StrongName" AfterTargets="Build" DependsOnTargets="_SetSnExe" Condition="'$(TargetPath)' != ''" Inputs="$(TargetPath)" Outputs="$(IntermediateOutputPath)Sn.stamp">
<Exec Command='"$(SnExe)" -R $(TargetPath) ..\xamarin.forms.snk' Condition=" '$(SignAssembly)' == 'true' " />
<Exec Command="&quot;$(SnExe)&quot; -R $(TargetPath) ..\xamarin.forms.snk" Condition=" '$(SignAssembly)' == 'true' " />
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes can probably be reverted.

Xamarin.Forms.Build.Tasks/ErrorMessages.Designer.cs Outdated Show resolved Hide resolved
@@ -5,7 +5,7 @@
<PropertyGroup>
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
<iOSPlatform Condition=" '$(iOSPlatform)' == '' ">iPhone</iOSPlatform>
<Platform Condition=" '$(Platform)' == '' ">$(iOSPlatform)</Platform>
<Platform Condition=" '$(Platform)' == '' ">iPhoneSimulator</Platform>
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this change intentional?

// DatePicker Format String: d, M, y
// Separator: "/"
[Test]
public void DatePickerOneDigit()
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like several of these tests follow the same general format - would it be possible to use the TestCase attribute and create fewer test methods?

Copy link
Contributor

@hartez hartez left a comment

Choose a reason for hiding this comment

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

A couple more things.

@@ -160,7 +162,31 @@ void UpdateDateFromModel(bool animate)
if (_picker.Date.ToDateTime().Date != Element.Date.Date)
_picker.SetDate(Element.Date.ToNSDate(), animate);

Control.Text = Element.Date.ToString(Element.Format);
//can't use Element.Format because it won't display the correct format if the region and language are set differently
if (Element.Format.Equals("") || Element.Format.Equals("d") || Element.Format.Equals("D"))
Copy link
Member

Choose a reason for hiding this comment

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

Do you think we should use something like string.IsNullOrWhitespace instead of .Equals("")?

@BurkusCat
Copy link
Contributor

BurkusCat commented Jul 28, 2020

I think the issue #11520 still exists after this PR. On UWP I tried using "HH:mm" but it does not use a leading 0 in the picker or in the display of the selected value:
image

image

I think the format the UWP picker is displaying is "H:mm".

@samhouts samhouts changed the base branch from 4.8.0 to 5.0.0 August 4, 2020 21:51
@samhouts samhouts added this to the 5.0.0 milestone Aug 4, 2020
@memu8 memu8 force-pushed the fix-datetimepickerlocalization branch from 1c7bf16 to ca5c809 Compare August 5, 2020 20:50
@PureWeen PureWeen added the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Nov 5, 2020
@PureWeen PureWeen force-pushed the fix-datetimepickerlocalization branch from 597c872 to e10cca9 Compare November 10, 2020 23:52
@rmarinho rmarinho merged commit 938700b into 5.0.0 Nov 11, 2020
@rmarinho rmarinho deleted the fix-datetimepickerlocalization branch November 11, 2020 15:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to Time Picker Set to Binding Value # DatePicker Inconsistencies # TimePicker Inconsistencies
7 participants