Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public virtual void ResetAppearance(BottomNavigationView bottomView)
{
bottomView.ItemIconTintList = GetDefaultTabColorList(_shellContext.AndroidContext);
bottomView.ItemTextColor = GetDefaultTabColorList(_shellContext.AndroidContext);
SetBackgroundColor(bottomView, null);
SetBackground(bottomView, null);
AndroidSystemChrome.UpdateBottomChrome(
bottomView,
new SolidColorBrush(ShellRenderer.DefaultBottomNavigationViewBackgroundColor));
Expand All @@ -52,6 +52,14 @@ public virtual void SetAppearance(BottomNavigationView bottomView, IShellAppeara
{
IShellAppearanceElement controller = appearance;
var backgroundColor = controller.EffectiveTabBarBackgroundColor;
var shellAppearance = appearance as ShellAppearance;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[major] Architectural Layer Placement — The precedence chain (TabBarBackgroundColorShell.BackgroundShell.BackgroundColor → default) is reconstructed by hand here and byte-for-byte duplicated in SafeShellTabBarAppearanceTracker.cs:74-80. Because it was removed from ShellAppearance (the shared layer) and re-added per platform, Windows/Tizen/ShellItemRenderer silently diverge (see the ShellAppearance.cs:82 comment). This resolution belongs on ShellAppearance as an internal EffectiveTabBarBackground brush so all platforms share one definition.

Also, appearance as ShellAppearance narrows an IShellAppearanceElement parameter to a concrete type: any other IShellAppearanceElement implementation now yields shellAppearance == null, so both the Background and BackgroundColor fallbacks silently disappear for it (previously the interface member supplied the BackgroundColor fallback for everyone).

var background = backgroundColor is not null
? new SolidColorBrush(backgroundColor)
: !Brush.IsNullOrEmpty(shellAppearance?.Background)
? shellAppearance.Background
: shellAppearance?.BackgroundColor is not null
? new SolidColorBrush(shellAppearance.BackgroundColor)
: null;
var foregroundColor = controller.EffectiveTabBarForegroundColor;
var disabledColor = controller.EffectiveTabBarDisabledColor;
var unselectedColor = controller.EffectiveTabBarUnselectedColor;
Expand All @@ -70,14 +78,21 @@ public virtual void SetAppearance(BottomNavigationView bottomView, IShellAppeara
bottomView.ItemTextColor = _itemTextColor;
bottomView.ItemIconTintList = _itemIconTint;

SetBackgroundColor(bottomView, backgroundColor);
SetBackground(bottomView, background);
AndroidSystemChrome.UpdateBottomChrome(
bottomView,
new SolidColorBrush(backgroundColor ?? ShellRenderer.DefaultBottomNavigationViewBackgroundColor));
background ?? new SolidColorBrush(ShellRenderer.DefaultBottomNavigationViewBackgroundColor));
}

protected virtual void SetBackgroundColor(BottomNavigationView bottomView, Color color)
protected virtual void SetBackground(BottomNavigationView bottomView, Brush brush)
{
if (brush is GradientBrush)
{
bottomView.UpdateBackground(brush);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[moderate] Performance-Critical Path / Native Platform Defaults Preservation — The gradient branch returns immediately, so unlike the solid path it has no "same value → early return" short-circuit. SetAppearance runs on every Shell appearance change (including tab/section switches), so a static gradient re-runs UpdateBackground and rebuilds the drawable on every tab switch. Cache the last applied brush and skip when unchanged, mirroring the lastColor == newColor guards below.

It also bypasses the MaterialShapeDrawable + InitializeElevationOverlay(bottomView.Context) path used for solid colors, so a gradient tab bar loses the Material elevation overlay in dark theme. If that is intentional, a short comment would prevent a future "fix" from re-adding it.

return;
}

var color = (brush as SolidColorBrush)?.Color;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness / Memory — Gradient → solid transition is not handled cleanly. After the gradient branch has installed a MauiDrawable via bottomView.UpdateBackground(brush), a subsequent solid/reset call reaches here where oldBackground as ColorDrawable and as ColorChangeRevealDrawable are both null, so lastColor falls back to Colors.Transparent and the ColorChangeRevealDrawable animates from transparent — a visible flash rather than a gradient→color transition. The previous MauiDrawable is also replaced by ViewCompat.SetBackground without being disposed (contrast with ViewExtensions.UpdateBackground, which disposes the outgoing MauiDrawable). Detect the gradient-drawable case explicitly and dispose/short-circuit the reveal animation.

#pragma warning disable XAOBS001 // Obsolete
var menuView = bottomView.GetChildAt(0) as BottomNavigationMenuView;
#pragma warning restore XAOBS001 // Obsolete
Expand Down Expand Up @@ -130,6 +145,12 @@ protected virtual void SetBackgroundColor(BottomNavigationView bottomView, Color
}
}

[Obsolete("Use SetBackground(BottomNavigationView, Brush) instead.")]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[major] Backward Compatibility / Public API Surfaceprotected virtual SetBackgroundColor is a shipped API (PublicAPI/net-android/PublicAPI.Shipped.txt:4369). After this change no call site invokes it any more (ResetAppearance line 45 and SetAppearance line 81 both call SetBackground directly), so every existing subclass that overrides SetBackgroundColor to customize the bottom-bar background is silently bypassed — it compiles, emits only an obsolete warning, and stops taking effect at runtime. That is a behavioral break for third-party Shell renderers shipping against a servicing branch (net11.0).

Suggested shape: keep SetBackground handling only the gradient case and delegate the solid/null case to the existing SetBackgroundColor virtual (wrapped in #pragma warning disable CS0618), so subclass overrides continue to be dispatched. That also avoids obsoleting a shipped protected member and avoids adding new permanent protected API for a bug fix.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[major] Backward CompatibilitySetBackgroundColor is a shipped protected virtual extension point, and SetAppearance/ResetAppearance no longer call it. Any existing subclass that overrides SetBackgroundColor to customize the bottom bar background silently stops being invoked after this change — the override compiles, produces only an obsoletion warning, and the customization vanishes at runtime. Consider having SetBackground route the non-gradient case through SetBackgroundColor (e.g. #pragma-suppressed call to the obsolete member) for one release so overrides keep working, or call this out explicitly as a breaking change in the PR description/release notes.

protected virtual void SetBackgroundColor(BottomNavigationView bottomView, Color color)
{
SetBackground(bottomView, color is not null ? new SolidColorBrush(color) : null);
}

static ColorStateList MakeDefaultColorStateList(Context context)
{
TypedValue mTypedValue = new TypedValue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,13 @@ void UpdateiOS15TabBarAppearance(UITabBarController controller, ShellAppearance
{
IShellAppearanceElement appearanceElement = appearance;

var backgroundColor = appearanceElement.EffectiveTabBarBackgroundColor;
var background = appearanceElement.EffectiveTabBarBackgroundColor is not null

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[moderate] Architectural Layer Placement — This 4-level precedence ternary is a verbatim copy of the one added in ShellBottomNavViewAppearanceTracker.SetAppearance (Android, line 56). Because the precedence now lives in the platform trackers rather than in ShellAppearance, the pre-iOS15 branch below (UpdateTabBarAppearance, line 114) and the Windows/Tizen handlers keep using the raw EffectiveTabBarBackgroundColor and get a different answer for the same Shell state. Resolve the precedence once in ShellAppearance (see comment on ShellAppearance.cs) and have every tracker consume it.

? new SolidColorBrush(appearanceElement.EffectiveTabBarBackgroundColor)
: !Brush.IsNullOrEmpty(appearance.Background)
? appearance.Background
: appearance.BackgroundColor is not null
? new SolidColorBrush(appearance.BackgroundColor)
: null;
var foregroundColor = appearanceElement.EffectiveTabBarForegroundColor;
var unselectedColor = appearanceElement.EffectiveTabBarUnselectedColor;
var titleColor = appearanceElement.EffectiveTabBarTitleColor;
Expand All @@ -84,7 +90,7 @@ void UpdateiOS15TabBarAppearance(UITabBarController controller, ShellAppearance
null,
foregroundColor ?? titleColor,
unselectedColor,
backgroundColor,
background,
titleColor ?? foregroundColor,
unselectedColor);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ void UpdateiOS15TabBarAppearance()
_defaultBarTextColor,
tabbed.IsSet(TabbedPage.SelectedTabColorProperty) ? tabbed.SelectedTabColor : null,
tabbed.IsSet(TabbedPage.UnselectedTabColorProperty) ? tabbed.UnselectedTabColor : null,
tabbed.IsSet(TabbedPage.BarBackgroundColorProperty) ? tabbed.BarBackgroundColor : null,
tabbed.IsSet(TabbedPage.BarBackgroundColorProperty) ? tabbed.BarBackgroundColor.AsPaint() : null,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[critical] Logic and CorrectnessColor.AsPaint() is an instance method on Color (src/Graphics/src/Graphics/Color.cs:166), not a null-tolerant extension method, so tabbed.BarBackgroundColor.AsPaint() throws NullReferenceException whenever BarBackgroundColor is null while IsSet(BarBackgroundColorProperty) is true.

IsSet is true for any explicit assignment, including an explicit null. Concrete repro: tabbed.BarBackgroundColor = Colors.Red; tabbed.BarBackgroundColor = null; (or BarBackgroundColor="{Binding BarColor}" / {AppThemeBinding Light=..., Dark={x:Null}} resolving to null) → crash on the next appearance update. Before this change the same input produced barBackgroundColor == null and fell back to defaultBarColor.

Suggest tabbed.BarBackgroundColor?.AsPaint() (and let the existing null => defaultBarColor switch arm handle it).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[critical] Logic and Correctness — Same NRE as TabbedPage.iOS.cs:849: Color.AsPaint() is an instance method, so tabbed.BarBackgroundColor.AsPaint() throws when BarBackgroundColorProperty is set to an explicit null (clearing the bar color, or a binding resolving to null). The previous code passed null and correctly restored _defaultBarColor. Use ?.AsPaint().

tabbed.IsSet(TabbedPage.BarTextColorProperty) ? tabbed.BarTextColor : null,
tabbed.IsSet(TabbedPage.BarTextColorProperty) ? tabbed.BarTextColor : null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ virtual Microsoft.Maui.Controls.Handlers.ShellItemHandler.CreateMoreBottomSheet(
virtual Microsoft.Maui.Controls.Handlers.ShellItemHandler.OnSectionChanged(Microsoft.Maui.Controls.ShellSection! shellSection, bool animate) -> void
virtual Microsoft.Maui.Controls.Handlers.ShellItemHandler.OnTabReselected(Microsoft.Maui.Controls.ShellSection! shellSection) -> void
virtual Microsoft.Maui.Controls.Handlers.ShellSectionHandler.OnCreateNavigationAnimation(Android.Content.Context! context, bool isPopping, bool enter) -> Android.Views.Animations.Animation?
~virtual Microsoft.Maui.Controls.Platform.Compatibility.ShellBottomNavViewAppearanceTracker.SetBackground(Google.Android.Material.BottomNavigation.BottomNavigationView bottomView, Microsoft.Maui.Controls.Brush brush) -> void

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[moderate] Public API Surface Design — This adds a permanent protected virtual API to a public unsealed compatibility type on a servicing branch (net11.0) in order to fix a rendering bug, while simultaneously obsoleting its shipped predecessor (PublicAPI.Shipped.txt:4369). Protected virtuals on ShellBottomNavViewAppearanceTracker cannot be removed later and become part of the compatibility-renderer contract.

If the gradient handling can be done inside the existing SetBackgroundColor dispatch path (see the ShellBottomNavViewAppearanceTracker.cs:148 comment), this API addition — and the obsoletion warning it forces on every existing subclass — can be avoided entirely.

virtual Microsoft.Maui.Controls.Platform.Compatibility.ShellFlyoutTemplatedContentRenderer.UpdateVerticalScrollMode() -> void
Microsoft.Maui.Controls.Handlers.ShellContentNavigationFragment
Microsoft.Maui.Controls.Handlers.ShellContentNavigationFragment.ShellContentNavigationFragment() -> void
Expand Down
2 changes: 1 addition & 1 deletion src/Controls/src/Core/Shell/ShellAppearance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public class ShellAppearance : IShellAppearanceElement
public double FlyoutHeight => _doubleArray[1];

Color IShellAppearanceElement.EffectiveTabBarBackgroundColor =>
TabBarBackgroundColor ?? BackgroundColor;
TabBarBackgroundColor;
Comment on lines 81 to +82

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[critical] Cross-Platform Behavioral Consistency / Regression Prevention — Dropping the ?? BackgroundColor fallback changes the semantics of the IShellAppearanceElement.EffectiveTabBarBackgroundColor interface contract for every consumer, but only two of the five call sites were compensated in this PR.

Uncompensated consumers that lose the Shell.BackgroundColor → tab bar fallback:

  • src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Windows.cs:763UpdateTopNavAreaBackground(null); Windows top-nav area reverts to the platform default when only Shell.BackgroundColor is set.
  • src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Tizen.cs:106.
  • src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellItemRenderer.cs:149 — now always writes DefaultBottomNavigationViewBackgroundColor into the existing ColorDrawable.
  • src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/SafeShellTabBarAppearanceTracker.cs:114 (UpdateTabBarAppearance, the pre-iOS-15 path) still reads EffectiveTabBarBackgroundColor directly and was not updated alongside the iOS-15 path at line 74.

Repro: a Shell with only Shell.BackgroundColor="Red" (no TabBarBackgroundColor, no Shell.Background) → red tab bar before, default tab bar after, on Windows/Tizen.

Prefer leaving EffectiveTabBarBackgroundColor untouched and adding a new EffectiveTabBarBackground (Brush) member that encodes the full precedence, so no existing consumer changes behavior.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[major] Regression Prevention / Cross-Platform Consistency — Dropping the ?? BackgroundColor fallback here changes behavior for every consumer of IShellAppearanceElement.EffectiveTabBarBackgroundColor, but the new TabBarBackgroundColor > Background > BackgroundColor precedence chain was only re-implemented in two of them (Android ShellBottomNavViewAppearanceTracker.SetAppearance and the iOS 15+ path of SafeShellTabBarAppearanceTracker). The remaining consumers now silently lose the Shell.BackgroundColor fallback with nothing replacing it:

  • Handlers/Shell/ShellItemHandler.Windows.cs:763Shell.BackgroundColor no longer tints the top nav/tab area on Windows.
  • Handlers/Shell/ShellItemHandler.Tizen.cs:106.
  • Compatibility/Handlers/Shell/iOS/SafeShellTabBarAppearanceTracker.cs:114 (UpdateTabBarAppearance, the pre-iOS15 path) and :30.
  • Compatibility/Handlers/Shell/Android/ShellItemRenderer.cs:149, which now paints the bottom view with DefaultBottomNavigationViewBackgroundColor when only Shell.BackgroundColor is set.
    Prefer exposing the resolved value on ShellAppearance itself (e.g. an internal EffectiveTabBarBackground brush implementing the full precedence) so all platforms share one implementation, instead of duplicating the ternary chain per-platform and regressing the untouched platforms.


Color IShellAppearanceElement.EffectiveTabBarDisabledColor =>
TabBarDisabledColor ?? DisabledColor;
Expand Down
2 changes: 1 addition & 1 deletion src/Controls/src/Core/TabbedPage/TabbedPage.iOS.cs
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,7 @@ void UpdateiOS15TabBarAppearance(UITabBar tabBar)
_defaultBarTextColor,
IsSet(SelectedTabColorProperty) ? SelectedTabColor : null,
IsSet(UnselectedTabColorProperty) ? UnselectedTabColor : null,
IsSet(BarBackgroundColorProperty) ? BarBackgroundColor : null,
IsSet(BarBackgroundColorProperty) ? BarBackgroundColor.AsPaint() : null,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[critical] Logic and Correctness — Same NullReferenceException as TabbedRenderer.cs:645. Color.AsPaint() is an instance method (src/Graphics/src/Graphics/Color.cs:166); IsSet(BarBackgroundColorProperty) is true for an explicitly-assigned null, so BarBackgroundColor.AsPaint() will NRE for BarBackgroundColor = null after a previous non-null assignment, or for a binding/AppThemeBinding that resolves to null.

Use BarBackgroundColor?.AsPaint().

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[critical] Logic and CorrectnessColor.AsPaint() is an instance method on Microsoft.Maui.Graphics.Color (Color.cs:166), not a null-safe extension. IsSet(BarBackgroundColorProperty) is true whenever the property has been assigned, including an explicit null (tabbedPage.BarBackgroundColor = null, or a binding that resolves to null after previously resolving to a color). In that case BarBackgroundColor.AsPaint() throws NullReferenceException and the tab bar update crashes. Before this change the same expression passed null through safely and UpdateiOS15TabBarAppearance fell back to defaultBarColor, restoring the platform default. Use BarBackgroundColor?.AsPaint() (and see the related stale-color issue in TabbedViewExtensions.cs:48).

IsSet(BarTextColorProperty) ? BarTextColor : null,
IsSet(BarTextColorProperty) ? BarTextColor : null);
}
Expand Down
12 changes: 11 additions & 1 deletion src/Controls/tests/TestCases.HostApp/Issues/Issue10445.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,16 @@ protected override void Init()
}
};

AddContentPage(page, "Home");
AddBottomTab(page, "Home");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[major] Regression Prevention and Test Coverage — This host page is shared by every UI-test leg, and adding a second bottom tab changes the rendering on all of them, but only 2 of the 5 committed baselines for ShellBackgroundSupportsGradientBrush were regenerated:

Updated: TestCases.iOS.Tests/snapshots/ios/, TestCases.iOS.Tests/snapshots/ios-26/.
Not updated (will fail): TestCases.Android.Tests/snapshots/android/, TestCases.Mac.Tests/snapshots/mac/, TestCases.WinUI.Tests/snapshots/windows/.

The try-fix run already observed an 8.58% Android screenshot delta from exactly this host-page change (try-fix/content.md, candidate 1). Regenerate the Android, Mac, and Windows baselines, or limit the new host layout to platforms with updated baselines.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[major] Regression Prevention — Switching AddContentPage to AddBottomTab (plus the new "Settings" tab) changes the rendered UI of Issue10445 on every platform, but only the iOS baselines (TestCases.iOS.Tests/snapshots/ios and ios-26) were regenerated. The existing baselines TestCases.Android.Tests/snapshots/android/ShellBackgroundSupportsGradientBrush.png, TestCases.WinUI.Tests/snapshots/windows/... and TestCases.Mac.Tests/snapshots/mac/... still show the old single-page layout, so ShellBackgroundSupportsGradientBrush will fail on Android/Windows/MacCatalyst. Regenerate all baselines — the Android one in particular is the only coverage for the ShellBottomNavViewAppearanceTracker half of this fix.

AddBottomTab(new ContentPage
Comment on lines +43 to +44

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[major] Regression Prevention and Test Coverage — Gate reported no new tests, and the only coverage added is a visual tweak to an existing screenshot test. The behaviors this PR actually changes are untested:

  1. Precedence: Shell.TabBarBackgroundColor must still win over a gradient Shell.Background (the new nested conditional in ShellBottomNavViewAppearanceTracker.cs:56-61 and SafeShellTabBarAppearanceTracker.cs:74-80).
  2. The removed EffectiveTabBarBackgroundColorBackgroundColor fallback: a Shell with only Shell.BackgroundColor set must still tint the tab bar (a device/unit test here would have caught the Windows/Tizen/ShellItemRenderer regression).
  3. Gradient → solid/cleared transition on both Android and iOS (see the TabbedViewExtensions.cs:57 comment).

A ShellAppearance-level unit test in Controls.Core.UnitTests covering the precedence matrix would be cheap and would pin the contract that the platform trackers now duplicate.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention — Coverage is limited to a single static screenshot of the "gradient wins" state. None of the transitions this PR actually changes are exercised: (a) TabBarBackgroundColor set → gradient must be ignored (precedence), (b) gradient → solid/default (ResetAppearance, iOS else branch at TabbedViewExtensions.cs:64), (c) TabbedPage.BarBackgroundColor set then cleared to null — which currently NREs on iOS (see TabbedPage.iOS.cs:849), and (d) tab re-selection/re-navigation re-running SetAppearance. At minimum add a case that toggles the background back to solid/null after the gradient, since that is where the stale-appearance and null-color bugs live.

{
Title = "Settings",
Content = new Label
{
Text = "Settings",
HorizontalOptions = LayoutOptions.Center,
VerticalOptions = LayoutOptions.Center
}
}, "Settings");
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
22 changes: 20 additions & 2 deletions src/Core/src/Platform/iOS/TabbedViewExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ internal static void UpdateiOS15TabBarAppearance(
UIColor? defaultBarTextColor,
Color? selectedTabColor,
Color? unselectedTabColor,
Color? barBackgroundColor,
Paint? barBackground,
Color? selectedBarTextColor,
Color? unSelectedBarTextColor)
{
Expand All @@ -45,7 +45,25 @@ internal static void UpdateiOS15TabBarAppearance(
_tabBarAppearance.ConfigureWithDefaultBackground();
}

var effectiveBarColor = (barBackgroundColor == null) ? defaultBarColor : barBackgroundColor.ToPlatform();
var effectiveBarColor = barBackground switch

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[moderate] Logic and Correctness — The switch loses the previous "no explicit background → use the platform default" behavior for two inputs:

  • SolidPaint with a null ColorsolidPaint.Color?.ToPlatform() yields null, and defaultBarColor is never applied. Previously barBackgroundColor == null mapped straight to defaultBarColor. This is reachable today via BarBackgroundColor.AsPaint() at TabbedRenderer.cs:645 / TabbedPage.iOS.cs:849 (see the NRE comments there — once null-guarded, this arm becomes the live path).
  • Any non-solid, non-gradient Paint (e.g. ImagePaint) → _ => null, and neither the gradient branch nor the color branch runs, so the tab bar keeps whatever appearance it had.

Consider SolidPaint { Color: null } => defaultBarColor and an explicit fallback for unsupported paint types.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[major] Logic and Correctness — The _ => null arm (and SolidPaint solidPaint => solidPaint.Color?.ToPlatform() when Color is null) makes effectiveBarColor null, which the if (effectiveBarColor != null) guard at line 68 treats as "do nothing" — not as "restore defaultBarColor". Two reachable regressions:

  1. Shell.Background = new ImageBrush{...} converts to ImageSourcePaint, hits _ => null, so no background is applied at all AND _tabBarAppearance.BackgroundColor keeps the previously applied color (the appearance object is cached in the ref field across calls). Android's UpdateBackground does render image paints, so the platforms diverge.
  2. Clearing a previously-set bar color (SolidColorBrush with a null Color, or BarBackgroundColor = null while IsSet is true) leaves the stale color on the tab bar instead of reverting to the platform default.
    Make the fallback explicit: SolidPaint sp => sp.Color?.ToPlatform() ?? defaultBarColor, and _ => defaultBarColor for non-gradient/unsupported paints (or handle them via tabBar.UpdateBackground).

{
null => defaultBarColor,
SolidPaint solidPaint => solidPaint.Color?.ToPlatform(),
_ => null,
};

if (barBackground is GradientPaint)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[moderate] Handler Mapper and Property Patterns / Cross-Platform Consistency — The gradient branch mutates _tabBarAppearance.BackgroundEffect = null and BackgroundColor = UIColor.Clear, but nothing restores those when the app later transitions gradient → solid/default. _tabBarAppearance is a cached ref field reused on the next call: taking the else branch removes the gradient layer, and if effectiveBarColor is null (default) the appearance is left with BackgroundEffect == null + BackgroundColor == UIColor.Clear, i.e. a transparent tab bar with no blur instead of the restored system default. Re-run ConfigureWithDefaultBackground() (or re-apply the captured defaultBarColor/effect) in the non-gradient branch. Note RemoveBackgroundLayer() is also called unconditionally on every appearance update, including the common no-gradient case.

{
_tabBarAppearance.BackgroundEffect = null;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[major] Native Platform Defaults Preservation — The gradient branch mutates _tabBarAppearance.BackgroundEffect = null, _tabBarAppearance.BackgroundColor = UIColor.Clear and tabBar.BackgroundColor = UIColor.Clear, but none of these is ever restored. ConfigureWithDefaultBackground() only runs once, when _tabBarAppearance is first allocated (line 42), and the else branch (line 64) only calls RemoveBackgroundLayer().

Repro: set a gradient Shell.Background / BarBackground, then clear it (or navigate to a page whose appearance has no background). barBackground becomes nulleffectiveBarColor = defaultBarColor; if defaultBarColor is null (the Shell tracker passes null for defaultBarColorSafeShellTabBarAppearanceTracker.cs:87), the if (effectiveBarColor != null) block is skipped and the tab bar is left permanently transparent with the default blur effect destroyed.

The else branch should also restore the default background (_tabBarAppearance.ConfigureWithDefaultBackground() or a captured default) alongside RemoveBackgroundLayer().

_tabBarAppearance.BackgroundColor = UIColor.Clear;
tabBar.BackgroundColor = UIColor.Clear;
tabBar.UpdateBackground(barBackground);
}
else
{
tabBar.RemoveBackgroundLayer();
}
Comment on lines +55 to +65

// Set BarBackgroundColor
if (effectiveBarColor != null)
{
Expand Down
Loading