Skip to content

[iOS] Auto Resize chrome icons on iOS to make it more consistent with other platforms - Toolbar items #30995

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

Jump to bottom
Merged
merged 4 commits into from
Aug 19, 2025
Merged

[iOS] Auto Resize chrome icons on iOS to make it more consistent with other platforms - Toolbar items #30995

merged 4 commits into from
Aug 19, 2025
+32 −2

Conversation

@kubaflo
Copy link
Contributor

@kubaflo kubaflo commented Aug 3, 2025

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Issues Fixed

Fixes #30994

Before After 
<Shell.ToolbarItems>
    <ToolbarItem IconImageSource="groceries.png"/>
    <ToolbarItem IconImageSource="dotnet_bot.png"/>
</Shell.ToolbarItems>

Copilot AI review requested due to automatic review settings August 3, 2025 09:05
@kubaflo kubaflo requested a review from a team as a code owner August 3, 2025 09:05
@kubaflo kubaflo self-assigned this Aug 3, 2025
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Aug 3, 2025
@dotnet-policy-service
Copy link
Contributor

Hey there @@kubaflo! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

Copilot AI reviewed Aug 3, 2025
View reviewed changes
Copy link
Contributor

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

This PR implements automatic resizing of toolbar item icons on iOS to maintain consistency with other platforms. The change ensures that oversized icons are scaled down to fit within the iOS navigation bar's 44-point height limit while preserving aspect ratio.

  • Adds a new ScaleImageToSystemDefaults method that intelligently resizes icons based on iOS navigation bar constraints
  • Updates both primary and secondary toolbar items to use the new scaling logic
  • Maintains special handling for FontImageSource icons that already have explicit size settings

src/Controls/src/Core/Compatibility/iOS/Extensions/ToolbarItemExtensions.cs
// so using the full height ensures maximum visual clarity and maintains consistency
// with iOS design standards. This allows icons to utilize the entire available
// vertical space within the navigation bar container.
var defaultIconHeight = 44f;
Copy link

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

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

The magic number 44f should be extracted to a named constant to improve maintainability and make the iOS navigation bar height constraint explicit.

Suggested change
var defaultIconHeight = 44f;
var defaultIconHeight = NavigationBarHeight;

Copilot uses AI. Check for mistakes.
src/Controls/src/Core/Compatibility/iOS/Extensions/ToolbarItemExtensions.cs
// with iOS design standards. This allows icons to utilize the entire available
// vertical space within the navigation bar container.
var defaultIconHeight = 44f;
var buffer = 0.1;
Copy link

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

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

The magic number 0.1 should be extracted to a named constant with a descriptive name explaining its purpose as a tolerance buffer for size comparison.

Suggested change
var buffer = 0.1;
var buffer = IconHeightComparisonTolerance;

Copilot uses AI. Check for mistakes.
src/Controls/src/Core/Compatibility/iOS/Extensions/ToolbarItemExtensions.cs
{
if (imageSource is not FontImageSource fontImageSource || !fontImageSource.IsSet(FontImageSource.SizeProperty))
{
icon = icon.ResizeImageSource(originalImageSize.Width, defaultIconHeight, originalImageSize);
Copy link

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

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

The ResizeImageSource call passes originalImageSize.Width for the new width but also passes originalImageSize as the third parameter. This appears redundant and could be confusing - consider clarifying the intent or simplifying the parameters.

Suggested change
icon = icon.ResizeImageSource(originalImageSize.Width, defaultIconHeight, originalImageSize);
icon = icon.ResizeImageSource(originalImageSize.Width, defaultIconHeight);

Copilot uses AI. Check for mistakes.
@kubaflo kubaflo added platform/ios area-controls-shell Shell Navigation, Routes, Tabs, Flyout area-controls-toolbar ToolBar and removed community ✨ Community Contribution labels Aug 3, 2025
@PureWeen
Copy link
Member

PureWeen commented Aug 3, 2025

/azp run

@PureWeen PureWeen added this to the .NET 10.0-rc1 milestone Aug 11, 2025
rmarinho
rmarinho previously approved these changes Aug 13, 2025
View reviewed changes
@github-project-automation github-project-automation bot moved this from Todo to Approved in MAUI SDK Ongoing Aug 13, 2025
@rmarinho
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@dotnet dotnet deleted a comment from azure-pipelines bot Aug 14, 2025
PureWeen
PureWeen requested changes Aug 14, 2025
View reviewed changes
src/Controls/src/Core/Compatibility/iOS/Extensions/ToolbarItemExtensions.cs Outdated
static UIImage ScaleImageToSystemDefaults(ImageSource imageSource, UIImage uIImage)
{
var icon = imageSource is not FontImageSource
? uIImage?.ImageWithRenderingMode(UIImageRenderingMode.AlwaysOriginal)
Copy link
Member

Choose a reason for hiding this comment

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

@kubaflo based on what we reverted for NET9 do we need to change this back for now so that we get the blue tint still by default for iOS?

@github-project-automation github-project-automation bot moved this from Approved to Changes Requested in MAUI SDK Ongoing Aug 14, 2025
@PureWeen
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen added the p/0 Work that we can't release without label Aug 15, 2025
@PureWeen PureWeen moved this from Changes Requested to Ready To Review in MAUI SDK Ongoing Aug 17, 2025
@dotnet dotnet deleted a comment from azure-pipelines bot Aug 18, 2025
@dotnet dotnet deleted a comment from PureWeen Aug 18, 2025
@github-project-automation github-project-automation bot moved this from Ready To Review to Approved in MAUI SDK Ongoing Aug 18, 2025
@PureWeen
Copy link
Member

  • failing tests unrelated

@PureWeen PureWeen merged commit 6e75f3e into dotnet:net10.0 Aug 19, 2025
127 of 129 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Done in MAUI SDK Ongoing Aug 19, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Sep 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@rmarinho rmarinho rmarinho approved these changes

@PureWeen PureWeen PureWeen approved these changes

@StephaneDelcroix StephaneDelcroix Awaiting requested review from StephaneDelcroix StephaneDelcroix was automatically assigned from dotnet/dotnet-maui-reviewers

Assignees

@kubaflo kubaflo

Labels

area-controls-shell Shell Navigation, Routes, Tabs, Flyout area-controls-toolbar ToolBar p/0 Work that we can't release without platform/ios

Projects

MAUI SDK Ongoing
Status: Done

Milestone

.NET 10.0-rc1

Development

Successfully merging this pull request may close these issues.

3 participants

@kubaflo @PureWeen @rmarinho