Skip to content

Conversation

kubaflo
Copy link
Contributor

@kubaflo kubaflo commented Aug 25, 2024

Issues Fixed

Fixes #24428

Screen.Recording.2024-08-26.at.01.04.52.mov

@kubaflo kubaflo requested a review from a team as a code owner August 25, 2024 23:14
@kubaflo kubaflo requested review from Eilon and tj-devel709 August 25, 2024 23:14
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Aug 25, 2024
@kubaflo kubaflo added area-navigation NavigationPage platform/ios partner/android Issues for the Android SDK and removed community ✨ Community Contribution labels Aug 25, 2024
@rmarinho
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@kubaflo kubaflo added the community ✨ Community Contribution label Sep 2, 2024
@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

/rebase

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented Nov 15, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

@kubaflo Could you rebase and fix the conflict? Thanks in advance.

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen dismissed jsuarezruiz’s stale review December 6, 2024 21:34

Snapshots updated

@PureWeen PureWeen added this to the .NET 9 SR3 milestone Dec 6, 2024
@PureWeen PureWeen requested a review from Copilot January 6, 2025 08:31
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Jan 6, 2025

/rebase

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.

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • src/Controls/tests/TestCases.HostApp/Issues/Issue24428.xaml: Language not supported
Comments suppressed due to low confidence (2)

src/Controls/src/Core/Compatibility/Handlers/NavigationPage/iOS/NavigationRenderer.cs:39

  • [nitpick] The variable name _currentBarBackgroundBrush is clear and consistent with existing naming conventions.
Brush _currentBarBackgroundBrush;

src/Controls/src/Core/Compatibility/Handlers/NavigationPage/iOS/NavigationRenderer.cs:268

  • Ensure that the new behavior introduced in UpdateBarBackground and RefreshBarBackground is covered by tests.
if(_currentBarBackgroundBrush is GradientBrush gb)

@PureWeen
Copy link
Member

PureWeen commented Jan 6, 2025

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

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.

Looking good, I think we can add a verify for the initial light mode just in case something breaks and CI suddenly goes dark mode by default. #defensivetesting

mattleibow
mattleibow previously approved these changes Jan 6, 2025
@PureWeen
Copy link
Member

PureWeen commented Jan 6, 2025

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Jan 7, 2025

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

NavigationBarBackgroundShouldChange is failing on android

@PureWeen PureWeen requested a review from Copilot January 8, 2025 18:06
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.

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • src/Controls/tests/TestCases.HostApp/Issues/Issue24428.xaml: Language not supported
Comments suppressed due to low confidence (2)

src/Controls/src/Core/Toolbar/Toolbar.Android.cs:108

  • Ensure that the new UpdateBarBackground method is covered by tests in TestCases.Shared.Tests.
void UpdateBarBackground()

src/Controls/src/Core/Compatibility/Handlers/NavigationPage/iOS/NavigationRenderer.cs:723

  • The new behavior introduced in OnBarBackgroundChanged is not covered by tests. Ensure that this method is tested to verify the functionality.
void OnBarBackgroundChanged(object sender, EventArgs e)

@PureWeen
Copy link
Member

PureWeen commented Jan 8, 2025

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen self-requested a review January 8, 2025 21:25
@PureWeen PureWeen merged commit 4c05cb8 into dotnet:main Jan 9, 2025
104 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Feb 9, 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.

AppThemeBinding BarBackground with Brush in NavigationPage not working
6 participants