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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds support for Seconds to TimePicker #16079

Merged
merged 5 commits into from
Jul 4, 2024
Merged

Adds support for Seconds to TimePicker #16079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d0e65b3
Adds seconds support to TimePicker.
begleysm Jun 10, 2024
a81260a
Updates TimePicker to support UseSeconds. Seconds are not displayed u…
begleysm Jun 20, 2024
ab9dbb4
Fixes & updates Unit Tests related to adding Seconds & UseSeconds to …
begleysm Jun 20, 2024
a3e2a16
Merge branch 'master' of https://github.com/AvaloniaUI/Avalonia
begleysm Jun 20, 2024
85495aa
Adds a simple TimePicker with seconds enabled to DateTimePickerPage.…
begleysm Jun 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 71 additions & 3 deletions samples/ControlCatalog/Pages/DateTimePickerPage.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,23 @@
</Panel>
</StackPanel>

<TextBlock FontSize="18">A TimePicker with seconds enabled.</TextBlock>
<StackPanel Orientation="Vertical">
<Border BorderBrush="{DynamicResource CatalogBaseLowColor}"
BorderThickness="1" Padding="15">
<TimePicker UseSeconds="True" />
</Border>
<Panel Background="{DynamicResource CatalogBaseLowColor}">
<TextBlock Padding="15">
<TextBlock.Text>
<x:String>
&lt;TimePicker UseSeconds="True" /&gt;
</x:String>
</TextBlock.Text>
</TextBlock>
</Panel>
</StackPanel>

<Border BorderBrush="{DynamicResource CatalogBaseLowColor}"
BorderThickness="1" Padding="15">
<TimePicker>
Expand All @@ -85,8 +102,8 @@
</DataValidationErrors.Error>
</TimePicker>
</Border>

<TextBlock FontSize="18">A TimePicker with minute increments specified.</TextBlock>
<TextBlock FontSize="18">A TimePicker with minute increment specified.</TextBlock>
<StackPanel Orientation="Vertical">
<Border BorderBrush="{DynamicResource CatalogBaseLowColor}"
BorderThickness="1" Padding="15">
Expand All @@ -96,7 +113,24 @@
<TextBlock Padding="15">
<TextBlock.Text>
<x:String>
&lt;TimePicker MinuteIncrement="15" /&gt;
&lt;TimePicker MinuteIncrement="15" SecondIncrement="30" /&gt;
</x:String>
</TextBlock.Text>
</TextBlock>
</Panel>
</StackPanel>

<TextBlock FontSize="18">A TimePicker with seconds enabled and minute &amp; second increments specified.</TextBlock>
<StackPanel Orientation="Vertical">
<Border BorderBrush="{DynamicResource CatalogBaseLowColor}"
BorderThickness="1" Padding="15">
<TimePicker UseSeconds="True" MinuteIncrement="15" SecondIncrement="30" />
</Border>
<Panel Background="{DynamicResource CatalogBaseLowColor}">
<TextBlock Padding="15">
<TextBlock.Text>
<x:String>
&lt;TimePicker UseSeconds="True" MinuteIncrement="15" SecondIncrement="30" /&gt;
</x:String>
</TextBlock.Text>
</TextBlock>
Expand Down Expand Up @@ -137,6 +171,40 @@
</Panel>
</StackPanel>

<TextBlock FontSize="18">A TimePicker using a 12-hour clock and seconds.</TextBlock>
<StackPanel Orientation="Vertical">
<Border BorderBrush="{DynamicResource CatalogBaseLowColor}"
BorderThickness="1" Padding="15">
<TimePicker ClockIdentifier="12HourClock" UseSeconds="True" />
</Border>
<Panel Background="{DynamicResource CatalogBaseLowColor}">
<TextBlock Padding="15">
<TextBlock.Text>
<x:String>
&lt;TimePicker ClockIdentifier="12HourClock" UseSeconds="True" /&gt;
</x:String>
</TextBlock.Text>
</TextBlock>
</Panel>
</StackPanel>

<TextBlock FontSize="18">A TimePicker using a 24-hour clock and seconds.</TextBlock>
<StackPanel Orientation="Vertical">
<Border BorderBrush="{DynamicResource CatalogBaseLowColor}"
BorderThickness="1" Padding="15">
<TimePicker ClockIdentifier="24HourClock" UseSeconds="True" />
</Border>
<Panel Background="{DynamicResource CatalogBaseLowColor}">
<TextBlock Padding="15">
<TextBlock.Text>
<x:String>
&lt;TimePicker ClockIdentifier="24HourClock" UseSeconds="True" /&gt;
</x:String>
</TextBlock.Text>
</TextBlock>
</Panel>
</StackPanel>

</StackPanel>
</StackPanel>
</UserControl>
2 changes: 1 addition & 1 deletion samples/ControlCatalog/Pages/DateTimePickerPage.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public DateTimePickerPage()
"Order of month, day, and year is dynamically set based on user date settings";

this.Get<TextBlock>("TimePickerDesc").Text = "Use a TimePicker to let users set a time in your app, for example " +
"to set a reminder. The TimePicker displays three controls for hour, minute, and AM / PM(if necessary).These controls " +
"to set a reminder. The TimePicker displays four controls for hour, minute, seconds(optional), and AM / PM(if necessary).These controls " +
"are easy to use with touch or mouse, and they can be styled and configured in several different ways. " +
"12 - hour or 24 - hour clock and visibility of AM / PM is dynamically set based on user time settings, or can be overridden.";

Expand Down
3 changes: 3 additions & 0 deletions src/Avalonia.Controls/DateTimePickers/DateTimePickerPanel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public enum DateTimePickerPanelType
Day,
Hour,
Minute,
Second,
TimePeriod //AM or PM
}

Expand Down Expand Up @@ -516,6 +517,8 @@ private string FormatContent(int value, DateTimePickerPanelType panelType)
return new TimeSpan(value, 0, 0).ToString(ItemFormat);
case DateTimePickerPanelType.Minute:
return new TimeSpan(0, value, 0).ToString(ItemFormat);
case DateTimePickerPanelType.Second:
return new TimeSpan(0, 0, value).ToString(ItemFormat);
case DateTimePickerPanelType.TimePeriod:
return value == MinimumValue ? TimeUtils.GetAMDesignator() : TimeUtils.GetPMDesignator();
default:
Expand Down
98 changes: 89 additions & 9 deletions src/Avalonia.Controls/DateTimePickers/TimePicker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@ namespace Avalonia.Controls
[TemplatePart("PART_FlyoutButtonContentGrid", typeof(Grid))]
[TemplatePart("PART_HourTextBlock", typeof(TextBlock))]
[TemplatePart("PART_MinuteTextBlock", typeof(TextBlock))]
[TemplatePart("PART_SecondTextBlock", typeof(TextBlock))]
[TemplatePart("PART_PeriodTextBlock", typeof(TextBlock))]
[TemplatePart("PART_PickerPresenter", typeof(TimePickerPresenter))]
[TemplatePart("PART_Popup", typeof(Popup))]
[TemplatePart("PART_SecondColumnDivider", typeof(Rectangle))]
[TemplatePart("PART_SecondPickerHost", typeof(Border))]
[TemplatePart("PART_ThirdPickerHost", typeof(Border))]
[TemplatePart("PART_ThirdColumnDivider", typeof(Rectangle))]
[TemplatePart("PART_ThirdPickerHost", typeof(Border))]
[TemplatePart("PART_FourthPickerHost", typeof(Border))]
[PseudoClasses(":hasnotime")]
public class TimePicker : TemplatedControl
{
Expand All @@ -32,12 +35,24 @@ public class TimePicker : TemplatedControl
/// </summary>
public static readonly StyledProperty<int> MinuteIncrementProperty =
AvaloniaProperty.Register<TimePicker, int>(nameof(MinuteIncrement), 1, coerce: CoerceMinuteIncrement);

/// <summary>
/// Defines the <see cref="SecondIncrement"/> property
/// </summary>
public static readonly StyledProperty<int> SecondIncrementProperty =
AvaloniaProperty.Register<TimePicker, int>(nameof(SecondIncrement), 1, coerce: CoerceSecondIncrement);

/// <summary>
/// Defines the <see cref="ClockIdentifier"/> property
/// </summary>
public static readonly StyledProperty<string> ClockIdentifierProperty =
AvaloniaProperty.Register<TimePicker, string>(nameof(ClockIdentifier), "12HourClock", coerce: CoerceClockIdentifier);

/// <summary>
/// Defines the <see cref="UseSeconds"/> property
/// </summary>
public static readonly StyledProperty<bool> UseSecondsProperty =
AvaloniaProperty.Register<TimePicker, bool>(nameof(UseSeconds), false, coerce: CoerceUseSeconds);

/// <summary>
/// Defines the <see cref="SelectedTime"/> property
Expand All @@ -52,11 +67,14 @@ public class TimePicker : TemplatedControl
private Border? _firstPickerHost;
private Border? _secondPickerHost;
private Border? _thirdPickerHost;
private Border? _fourthPickerHost;
private TextBlock? _hourText;
private TextBlock? _minuteText;
private TextBlock? _secondText;
private TextBlock? _periodText;
private Rectangle? _firstSplitter;
private Rectangle? _secondSplitter;
private Rectangle? _thirdSplitter;
private Grid? _contentGrid;
private Popup? _popup;

Expand Down Expand Up @@ -85,6 +103,23 @@ private static int CoerceMinuteIncrement(AvaloniaObject sender, int value)

return value;
}

/// <summary>
/// Gets or sets the second increment in the picker
/// </summary>
public int SecondIncrement
{
get => GetValue(SecondIncrementProperty);
set => SetValue(SecondIncrementProperty, value);
}

private static int CoerceSecondIncrement(AvaloniaObject sender, int value)
{
if (value < 1 || value > 59)
throw new ArgumentOutOfRangeException(null, "1 >= SecondIncrement <= 59");

return value;
}

/// <summary>
/// Gets or sets the clock identifier, either 12HourClock or 24HourClock
Expand All @@ -103,6 +138,24 @@ private static string CoerceClockIdentifier(AvaloniaObject sender, string value)

return value;
}

/// <summary>
/// Gets or sets the use seconds switch, either true or false
/// </summary>
public bool UseSeconds
{

get => GetValue(UseSecondsProperty);
set => SetValue(UseSecondsProperty, value);
}

private static bool CoerceUseSeconds(AvaloniaObject sender, bool value)
{
if (!(value == true || value == false))
throw new ArgumentException("Invalid UseSeconds", default(bool).ToString());

return value;
}

/// <summary>
/// Gets or sets the selected time. Can be null.
Expand Down Expand Up @@ -135,13 +188,16 @@ protected override void OnApplyTemplate(TemplateAppliedEventArgs e)
_firstPickerHost = e.NameScope.Find<Border>("PART_FirstPickerHost");
_secondPickerHost = e.NameScope.Find<Border>("PART_SecondPickerHost");
_thirdPickerHost = e.NameScope.Find<Border>("PART_ThirdPickerHost");
_fourthPickerHost = e.NameScope.Find<Border>("PART_FourthPickerHost");

_hourText = e.NameScope.Find<TextBlock>("PART_HourTextBlock");
_minuteText = e.NameScope.Find<TextBlock>("PART_MinuteTextBlock");
_secondText = e.NameScope.Find<TextBlock>("PART_SecondTextBlock");
_periodText = e.NameScope.Find<TextBlock>("PART_PeriodTextBlock");

_firstSplitter = e.NameScope.Find<Rectangle>("PART_FirstColumnDivider");
_secondSplitter = e.NameScope.Find<Rectangle>("PART_SecondColumnDivider");
_thirdSplitter = e.NameScope.Find<Rectangle>("PART_ThirdColumnDivider");

_contentGrid = e.NameScope.Find<Grid>("PART_FlyoutButtonContentGrid");

Expand All @@ -160,7 +216,9 @@ protected override void OnApplyTemplate(TemplateAppliedEventArgs e)
_presenter.Dismissed += OnDismissPicker;

_presenter[!TimePickerPresenter.MinuteIncrementProperty] = this[!MinuteIncrementProperty];
_presenter[!TimePickerPresenter.SecondIncrementProperty] = this[!SecondIncrementProperty];
_presenter[!TimePickerPresenter.ClockIdentifierProperty] = this[!ClockIdentifierProperty];
_presenter[!TimePickerPresenter.UseSecondsProperty] = this[!UseSecondsProperty];
}
}

Expand All @@ -172,11 +230,20 @@ protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs chang
{
SetSelectedTimeText();
}
else if (change.Property == SecondIncrementProperty)
{
SetSelectedTimeText();
}
else if (change.Property == ClockIdentifierProperty)
{
SetGrid();
SetSelectedTimeText();
}
else if (change.Property == UseSecondsProperty)
{
SetGrid();
SetSelectedTimeText();
}
else if (change.Property == SelectedTimeProperty)
{
var (oldValue, newValue) = change.GetOldAndNewValue<TimeSpan?>();
Expand All @@ -196,29 +263,40 @@ private void SetGrid()
columnsD.Add(new ColumnDefinition(GridLength.Star));
columnsD.Add(new ColumnDefinition(GridLength.Auto));
columnsD.Add(new ColumnDefinition(GridLength.Star));
if (UseSeconds)
{
columnsD.Add(new ColumnDefinition(GridLength.Auto));
columnsD.Add(new ColumnDefinition(GridLength.Star));
}
if (!use24HourClock)
{
columnsD.Add(new ColumnDefinition(GridLength.Auto));
columnsD.Add(new ColumnDefinition(GridLength.Star));
}

_contentGrid.ColumnDefinitions = columnsD;

_thirdPickerHost!.IsVisible = UseSeconds;
_secondSplitter!.IsVisible = UseSeconds;

_thirdPickerHost!.IsVisible = !use24HourClock;
_secondSplitter!.IsVisible = !use24HourClock;
_fourthPickerHost!.IsVisible = !use24HourClock;
_thirdSplitter!.IsVisible = !use24HourClock;

var amPmColumn = (UseSeconds) ? 6 : 4;

Grid.SetColumn(_firstPickerHost!, 0);
Grid.SetColumn(_secondPickerHost!, 2);

Grid.SetColumn(_thirdPickerHost, use24HourClock ? 0 : 4);
Grid.SetColumn(_thirdPickerHost!, UseSeconds ? 4 : 0);
Grid.SetColumn(_fourthPickerHost, use24HourClock ? 0 : amPmColumn);

Grid.SetColumn(_firstSplitter!, 1);
Grid.SetColumn(_secondSplitter, use24HourClock ? 0 : 3);
Grid.SetColumn(_secondSplitter!, UseSeconds ? 3 : 0);
Grid.SetColumn(_thirdSplitter, use24HourClock ? 0 : amPmColumn-1);
}

private void SetSelectedTimeText()
{
if (_hourText == null || _minuteText == null || _periodText == null)
if (_hourText == null || _minuteText == null || _secondText == null || _periodText == null)
return;

var time = SelectedTime;
Expand All @@ -230,11 +308,12 @@ private void SetSelectedTimeText()
{
var hr = newTime.Hours;
hr = hr > 12 ? hr - 12 : hr == 0 ? 12 : hr;
newTime = new TimeSpan(hr, newTime.Minutes, 0);
newTime = new TimeSpan(hr, newTime.Minutes, newTime.Seconds);
Copy link
Contributor

@VisualMelon VisualMelon Aug 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The current behaviour with a time that includes seconds when UseSeconds = false is to keep the seconds, which may be misleading, particular as the time span defaults to the current time, so a user can select a time, but be off by 0-59 seconds without knowing it. (This is a breaking change)

I think it would be better to exclude the seconds when UseSeconds = false; I'm not sure what would be the right behaviour when setting UseSeconds = true having already been false, but my instinct is that it should be cleared to zero.

(I'm happy to work on this feature if useful; may suffice to update OnConfirmed in TimePickerPresenter; this comment is not attached to a relevant piece of code)

Copy link
Contributor Author

@begleysm begleysm Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Good catch.

Interestingly (in the absence of it being set manually with UseSeconds == true), newTime.Seconds appears to be set to be equal to the current time when the Control is first clicked on (not when it is first loaded or when the check button is clicked to set the time) and then stays that way forever (doesn't update even when the Control is clicked again or another control is clicked and then this control is clicked again). I'm not, currently, entirely sure why this is and it also isn't super relevant to this particular issue.

Changing the modified line to

newTime = (UseSeconds) ? 
                        new TimeSpan(hr, newTime.Minutes, newTime.Seconds) :
                        new TimeSpan(hr, newTime.Minutes, 0);

causes 0 to be used for seconds when UseSeconds == false & the selected seconds to be used for seconds when UseSeconds == true. But it doesn't, alone, address the case where UseSeconds switches from false to true.

Regarding when UseSeconds == false changes to UseSeconds == true. That is also a good edge case you've identified. We can detect this case in the UseSeconds portion of the OnPropertyChanged override method.
We can detect that UseSecond went from false to true and pass an argument to SetSelectedTimeText() to indicate that 0 should be used for seconds.

It could look something like this which should result in

  • using a 0 for seconds if UseSeconds == false
  • using a 0 for seconds if UseSeconds == true but we came this method because UseSeconds went from false to true
  • otherwise (if UseSeconds == true but was not just changed from false) using the SelectedTime.Seconds for seconds
    image

Copy link
Contributor

Choose a reason for hiding this comment

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

Having messed around a bit, I'm still not sure what the correct behaviour should be. I'm happy enough with the behaviour of us setting the seconds to zero when completing an interaction; trivial change is in VisualMelon@7e4df0f along with trivial example. It doesn't exercise the scenario where you have another control/piece of code updating the time while UseSeconds is false, and then setting it to true, but again, not sure what the behaviour should be, so may well warrant something more interesting

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I've not been more active on this; we should probably create an issue for it, as it is a serious breaking change and don't want it to get lost: I'll try to remember to do so on the weekend if no-one beats me to it.

}

_hourText.Text = newTime.ToString("%h");
_minuteText.Text = newTime.ToString("mm");
_secondText.Text = newTime.ToString("ss");
PseudoClasses.Set(":hasnotime", false);

_periodText.Text = time.Value.Hours >= 12 ? TimeUtils.GetPMDesignator() : TimeUtils.GetAMDesignator();
Expand All @@ -244,6 +323,7 @@ private void SetSelectedTimeText()
// By clearing local value, we reset text property to the value from the template.
_hourText.ClearValue(TextBlock.TextProperty);
_minuteText.ClearValue(TextBlock.TextProperty);
_secondText.ClearValue(TextBlock.TextProperty);
PseudoClasses.Set(":hasnotime", true);

_periodText.Text = DateTime.Now.Hour >= 12 ? TimeUtils.GetPMDesignator() : TimeUtils.GetAMDesignator();
Expand Down
Loading
Loading