Skip to content

[Windows] Fix for FlyoutItem in overflow menu not fully interactable #27575

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 7 commits into from
May 22, 2025
Merged

[Windows] Fix for FlyoutItem in overflow menu not fully interactable #27575

merged 7 commits into from
May 22, 2025

Conversation

TamilarasanSF4853
Copy link
Contributor

Root cause:

  • The issue arises because the NavigationViewItemViewModel does not set a valid background color for the overflow menu items. Instead, it assigns a null value to the background, which causes the space around the items to be unclickable and prevents the hover shadow effect from appearing. This results in a poor user experience when interacting with the overflow menu.

Description of change:

  • I updated the Background of the NavigationViewItemViewModel to ensure it always defaults to Transparent when no specific background value (either SelectedBackground or UnselectedBackground) is set. This prevents the Background from being null, which could cause hit-testing and rendering issues in the overflow menu.

Tested the behavior in the following platforms.

  • Android
  • Windows
  • iOS
  • Mac

Issues Fixed

Fixes - #23803

Output

Before Fix After Fix
Screen.Recording.2025-01-27.190134.mp4
Screen.Recording.2025-01-27.184055.mp4

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Feb 5, 2025
@TamilarasanSF4853
Copy link
Contributor Author

@dotnet-policy-service agree company="Syncfusion, Inc."

@karthikraja-arumugam karthikraja-arumugam added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Feb 5, 2025
@TamilarasanSF4853 TamilarasanSF4853 marked this pull request as ready for review February 5, 2025 12:59
@Copilot Copilot AI review requested due to automatic review settings February 5, 2025 12:59
@TamilarasanSF4853 TamilarasanSF4853 requested a review from a team as a code owner February 5, 2025 12:59
Copilot
Copilot AI reviewed Feb 5, 2025
View reviewed changes
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 3 out of 3 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue23803.cs:17

  • [nitpick] The test method name 'VerifyClickAroundOverflowMenuItem' could be more descriptive. Consider renaming it to 'VerifyOverflowMenuItemClickBehavior'.
public void VerifyClickAroundOverflowMenuItem()

@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz
jsuarezruiz reviewed Feb 5, 2025
View reviewed changes
src/Core/src/Platform/Windows/NavigationViewItemViewModel.cs Outdated
@@ -110,7 +111,7 @@ public WBrush? Foreground

public WBrush? Background
{
get => IsSelected ? SelectedBackground : UnselectedBackground;
get => (IsSelected ? SelectedBackground : UnselectedBackground) ?? new SolidColorBrush(Microsoft.UI.Colors.Transparent);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you include a comment explaining why requires a default background. That context could be useful for future reviewers etc.

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 Background color is set to null since both SelectedBackground and UnselectedBackground return null. Adding a default transparent background ensures it is never null, preventing rendering inconsistencies.

@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

/rebase

@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz
jsuarezruiz requested changes Mar 11, 2025
View reviewed changes
Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

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

The test DynamicTabSectionVisibility is failing on Windows with some differences in the snapshot.
image

Could you review it and verify if is related with the changes?

@TamilarasanSF4853
Copy link
Contributor Author

The test DynamicTabSectionVisibility is failing on Windows with some differences in the snapshot. image

Could you review it and verify if is related with the changes?

@jsuarezruiz I ran the issue locally and identified the cause of the test failure. The failure occurs because, before the fix, the mouse clicked on the right side of the tab item text, whereas after the fix, it clicks directly over the tab item text. This change in click behavior affects the appearance of the blue tab indicator. When clicking over the text, the indicator does not appear, but when clicking beside the text, it does. I also tested the behavior without the fix and observed that the indicator only appears when clicking outside the text.
Now, I have committed the updated CI image for Windows, please kindly review and share your concerns.

@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz
jsuarezruiz requested changes Mar 25, 2025
View reviewed changes
Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

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

The test DynamicTabSectionVisibility has failed on Windows in more than 1 retry with the branch. That makes me think it might be related to the changes.

Could you review it?

@jfversluis jfversluis added the area-controls-shell Shell Navigation, Routes, Tabs, Flyout label Mar 25, 2025
@TamilarasanSF4853
Copy link
Contributor Author

The test DynamicTabSectionVisibility has failed on Windows in more than 1 retry with the branch. That makes me think it might be related to the changes.

Could you review it?

@jsuarezruiz The failure in DynamicTabSectionVisibility seems to be related to PR #25550, rather than my changes. I have updated to the new CI image accordingly.

@jsuarezruiz
Copy link
Contributor

/rebase

@jsuarezruiz
Copy link
Contributor

/azp run MAUI-UITests-public

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@PureWeen PureWeen changed the base branch from main to inflight/current May 22, 2025 12:23
@PureWeen PureWeen merged commit 5d6d9f1 into dotnet:inflight/current May 22, 2025
77 checks passed
github-actions bot pushed a commit that referenced this pull request May 22, 2025
@TamilarasanSF4853 @PureWeen
[Windows] Fix for FlyoutItem in overflow menu not fully interactable (#…
584630a 
…27575)

* fixed shell items click issue

* fix for shell items click issue

* fix for flyoutitem in the overflow menu is not fully intractable

* fix for flyoutitem in the overflow menu is not fully intractable

* Update Issue6784.cs

Removed test case fails on Windows

* added new image

* added new ci image
github-actions bot pushed a commit that referenced this pull request May 30, 2025
@TamilarasanSF4853 @rmarinho
[Windows] Fix for FlyoutItem in overflow menu not fully interactable (#…
afd836a 
…27575)

* fixed shell items click issue

* fix for shell items click issue

* fix for flyoutitem in the overflow menu is not fully intractable

* fix for flyoutitem in the overflow menu is not fully intractable

* Update Issue6784.cs

Removed test case fails on Windows

* added new image

* added new ci image
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@jsuarezruiz jsuarezruiz jsuarezruiz approved these changes

@StephaneDelcroix StephaneDelcroix Awaiting requested review from StephaneDelcroix StephaneDelcroix is a code owner automatically assigned from dotnet/dotnet-maui-reviewers

@PureWeen PureWeen Awaiting requested review from PureWeen PureWeen is a code owner automatically assigned from dotnet/dotnet-maui-reviewers

Assignees
No one assigned
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@TamilarasanSF4853 @jsuarezruiz @jfversluis @PureWeen @karthikraja-arumugam