Skip to content

Apply nullability attributes to TimeChangedEventArgs #29572

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.

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

Open
wants to merge 2 commits into
base: net10.0
Choose a base branch
from

Conversation

jsuarezruiz
Copy link
Contributor

Description of Change

In .NET 10 we make the Time property of the TimePicker class nullable #27930 However missed to apply nullability attributes to TimeChangedEventArgs. With the changes from this PR we align the behavior with the DatePicker https://github.com/dotnet/maui/blob/net10.0/src/Controls/src/Core/DateChangedEventArgs.cs

Issues Fixed

Fixes #29513
Related with #27930

@Copilot Copilot AI review requested due to automatic review settings May 19, 2025 10:59
@jsuarezruiz jsuarezruiz added the area-controls-datetimepicker DatePicker, TimePicker label May 19, 2025
@jsuarezruiz jsuarezruiz requested a review from a team as a code owner May 19, 2025 10:59
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

The purpose of the PR is to apply nullability attributes to TimeChangedEventArgs in .NET 10 so that its behavior aligns with DatePicker. The key changes are:

  • Updating the constructor signature to accept nullable TimeSpan values.
  • Changing the public properties OldTime and NewTime to be nullable.
  • Propagating these API changes to all PublicAPI.Unshipped.txt files for various platforms.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Controls/src/Core/TimeChangedEventArgs.cs Updated API to use nullable TimeSpan for constructor and properties.
src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt Replaced non-nullable entries with nullable counterparts for TimeChangedEventArgs.
src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt Replaced non-nullable entries with nullable counterparts for TimeChangedEventArgs.
src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt Replaced non-nullable entries with nullable counterparts for TimeChangedEventArgs.
src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt Replaced non-nullable entries with nullable counterparts for TimeChangedEventArgs.
src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt Replaced non-nullable entries with nullable counterparts for TimeChangedEventArgs.
src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt Replaced non-nullable entries with nullable counterparts for TimeChangedEventArgs.

@@ -88,8 +88,8 @@ public void DatePickerSelectedEventArgs(TimeSpan initialTime, TimeSpan finalTime
timePicker.Time = initialTime;

TimePicker pickerFromSender = null;
TimeSpan oldTime = new TimeSpan();
TimeSpan newTime = new TimeSpan();
TimeSpan? oldTime = new TimeSpan();
Copy link
Member

Choose a reason for hiding this comment

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

Should it be initialized now at null

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could be. In this case following the same pattern done with the changes in DatePicker (handler, tests etc): https://github.com/dotnet/maui/blob/net10.0/src/Controls/tests/Core.UnitTests/DatePickerUnitTest.cs#L179

Copy link
Member

Choose a reason for hiding this comment

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

Ok, we might want to create a couple of more tests with these variables taking null and everything works too.

@jsuarezruiz jsuarezruiz requested a review from rmarinho May 20, 2025 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-datetimepicker DatePicker, TimePicker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants