Skip to content

[iOS] DatePicker | Remove unnecessary UpdateDate() call in UpdateTextColor() #27811

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

Merged
merged 10 commits into from
Apr 22, 2025

Conversation

bhavanesh2001
Copy link
Contributor

Description of Change

This change removes an unnecessary call to UpdateDate() from UpdateTextColor() for iOS.

Root Cause

  • UpdateTextColor() was incorrectly triggering UpdateDate(), which overrode the expected locale-based date format with the virtual view's default format
  • As a result, the DatePicker initially displayed the wrong format instead of following the device's locale settings
  • However, text color updates do not require a date update, and this call was redundant.

Fix

  • Removed the unnecessary UpdateDate() invocation inside UpdateTextColor().
  • Verified that text color changes are still applied correctly without triggering UpdateDate().

Issues Fixed

Fixes #27803

@bhavanesh2001 bhavanesh2001 requested a review from a team as a code owner February 14, 2025 19:41
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Feb 14, 2025
@bhavanesh2001 bhavanesh2001 changed the title [iOS] Remove unnecessary UpdateDate() call in UpdateTextColor() [iOS] DatePicker | Remove unnecessary UpdateDate() call in UpdateTextColor() Feb 14, 2025
Copy link
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

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

The comment does say it was needed. Can you add a test where changing the color will still update on the UI?

@bhavanesh2001 bhavanesh2001 force-pushed the ios_datepicker_fomat branch 2 times, most recently from eac116b to 0f17f8f Compare February 15, 2025 04:59
@bhavanesh2001
Copy link
Contributor Author

bhavanesh2001 commented Feb 15, 2025

@rmarinho I added a test. However , text color is a styling property, and Appium does not provide a way to verify it directly. The current test confirms that the binding updates correctly, but it does not guarantee that the UI visually reflects the color change.
If i'm missing something here please let me know. That said, the UI is updating as expected.

Edit: Never mind I figured it's need to be verified comparing snapshots.

Screen.Recording.mov

@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented Feb 17, 2025

/azp run

This comment was marked as off-topic.

@bhavanesh2001
Copy link
Contributor Author

/rebase

@github-actions github-actions bot force-pushed the ios_datepicker_fomat branch from 35716ba to b302eb0 Compare February 20, 2025 10:03
@bhavanesh2001
Copy link
Contributor Author

@jfversluis Could you run pipelines?

Copy link

Azure Pipelines successfully started running 3 pipeline(s).


public override string Issue => "DatePicker default format on iOS";

#if !MACCATALYST
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The DatePicker seems inconsistent on Mac. The TextColor property definitely isn't working. Check #20904

Copy link
Member

Choose a reason for hiding this comment

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

Are you able to wrap the whole file with a #if TEST_FAILS_ON_CATALYST?

See

#if TEST_FAILS_ON_CATALYST // This test fails on catalyst, unable to click the back button as its does not have an identifier.

I am not sure if we have something that is watching files for these lines to give us stats or something. AI or someting maybe.

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@rmarinho
Copy link
Member

/rebase

@github-actions github-actions bot force-pushed the ios_datepicker_fomat branch from 8f9ee10 to 8f70f14 Compare February 27, 2025 15:24
@rmarinho
Copy link
Member

rmarinho commented Feb 27, 2025

/azp run

This comment was marked as outdated.

@jfversluis jfversluis moved this from Todo to Ready To Review in MAUI SDK Ongoing Mar 7, 2025
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jfversluis jfversluis self-assigned this Mar 7, 2025
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

Code looks good. Nice test.

I think there may be some tech involved with the way to exclue failing tests on some platforms. Added a comment.


public override string Issue => "DatePicker default format on iOS";

#if !MACCATALYST
Copy link
Member

Choose a reason for hiding this comment

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

Are you able to wrap the whole file with a #if TEST_FAILS_ON_CATALYST?

See

#if TEST_FAILS_ON_CATALYST // This test fails on catalyst, unable to click the back button as its does not have an identifier.

I am not sure if we have something that is watching files for these lines to give us stats or something. AI or someting maybe.

@github-project-automation github-project-automation bot moved this from Ready To Review to Changes Requested in MAUI SDK Ongoing Mar 12, 2025
@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen moved this from Changes Requested to Ready To Review in MAUI SDK Ongoing Mar 18, 2025
@PureWeen PureWeen modified the milestones: .NET 9 SR6, .NET 9 SR7 Mar 24, 2025
@dotnetaspire
Copy link

This didn't go with 9.6? 😔

@rmarinho rmarinho moved this from Ready To Review to Approved in MAUI SDK Ongoing Apr 14, 2025
@PureWeen PureWeen changed the base branch from main to inflight/current April 22, 2025 22:09
@PureWeen PureWeen merged commit 685914d into dotnet:inflight/current Apr 22, 2025
128 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Done in MAUI SDK Ongoing Apr 22, 2025
PureWeen pushed a commit that referenced this pull request Apr 23, 2025
…Color() (#27811)

* remove excessive UpdateDate() call

* adding Ui test

* update UItest

* adding snapshots

* add a static date in test case

* match date with snapshots

* change preprocessor directive

* add comment
github-actions bot pushed a commit that referenced this pull request Apr 28, 2025
…Color() (#27811)

* remove excessive UpdateDate() call

* adding Ui test

* update UItest

* adding snapshots

* add a static date in test case

* match date with snapshots

* change preprocessor directive

* add comment
anandhan-rajagopal pushed a commit to anandhan-rajagopal/maui that referenced this pull request May 2, 2025
…Color() (dotnet#27811)

* remove excessive UpdateDate() call

* adding Ui test

* update UItest

* adding snapshots

* add a static date in test case

* match date with snapshots

* change preprocessor directive

* add comment
anandhan-rajagopal pushed a commit to anandhan-rajagopal/maui that referenced this pull request May 2, 2025
…Color() (dotnet#27811)

* remove excessive UpdateDate() call

* adding Ui test

* update UItest

* adding snapshots

* add a static date in test case

* match date with snapshots

* change preprocessor directive

* add comment
PureWeen pushed a commit that referenced this pull request May 2, 2025
…Color() (#27811)

* remove excessive UpdateDate() call

* adding Ui test

* update UItest

* adding snapshots

* add a static date in test case

* match date with snapshots

* change preprocessor directive

* add comment
SuthiYuvaraj pushed a commit to SuthiYuvaraj/maui that referenced this pull request May 9, 2025
…Color() (dotnet#27811)

* remove excessive UpdateDate() call

* adding Ui test

* update UItest

* adding snapshots

* add a static date in test case

* match date with snapshots

* change preprocessor directive

* add comment
@github-actions github-actions bot locked and limited conversation to collaborators May 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

DatePicker default format on iOS
7 participants