Skip to content
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

Fix Shell Flyout Menu Visibility Issue When Navigating Between TabBar and Flyout Items #25510

Merged
merged 5 commits into from
Nov 27, 2024
Merged

Fix Shell Flyout Menu Visibility Issue When Navigating Between TabBar and Flyout Items #25510

merged 5 commits into from
Nov 27, 2024

Conversation

vishnumenon2684
Copy link
Contributor

@vishnumenon2684 vishnumenon2684 commented Oct 24, 2024 •
edited
Loading

Issue:

In the sample, if there are both flyout items and tab bar items in the Shell, navigating to a TabBar item causes the flyout menu icon to disappear. However, when navigating back to a flyout item, the flyout menu is not visible.

Root Cause:

When navigating to a TabBar item, the flyout behavior is set to Disabled. Upon returning to a flyout item, the flyout behavior is not restored. This issue was previously working fine but regressed due to PR-23863. In that PR, navigating using services clears the previous sections content cache, so when navigating back to the flyout page, the page is unavailable, preventing the flyout behavior from being reset.

Fix:

We have updated the NotifyFlyoutBehaviorObservers of the shell when disconnect is set to false to ensure that the flyout behavior is properly updated when the page is available

Fixes #25436

@vishnumenon2684 vishnumenon2684 requested a review from a team as a code owner October 24, 2024 17:13
@vishnumenon2684 vishnumenon2684 requested review from mattleibow and jsuarezruiz and removed request for a team October 24, 2024 17:13
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

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

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Oct 24, 2024
@PureWeen
Copy link
Member

/backport to main

@github-actions GitHub Actions
Copy link
Contributor

Started backporting to main: https://github.com/dotnet/maui/actions/runs/11504316344

@github-actions github-actions bot mentioned this pull request Oct 24, 2024
Closed
PureWeen
PureWeen requested changes Oct 24, 2024
View reviewed changes
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.

the page is unavailable, preventing the flyout behavior from being reset.

I think with the way flyout behavior calculation works the fix here my be that we need to refire this Notify method once the CurrentPage becomes available again.

The FlyoutBehavior value takes into account if the FlyoutBehavior is set on the Page itself which I think might cause this to miscalculate if the user has set FlyoutBehavior on the realized ContentPage

@PureWeen PureWeen added this to the .NET 9.0 GA milestone Oct 24, 2024
@Redth Redth modified the milestones: .NET 9.0 GA, .NET 9 SR1 Oct 29, 2024
@karthikraja-arumugam
Copy link
Contributor

The FlyoutBehavior value takes into account if the FlyoutBehavior is set on the Page itself which I think might cause this to miscalculate if the user has set FlyoutBehavior on the realized ContentPage

@PureWeen, based on the suggestion I'm looking for an alternate fix.

@dotnet dotnet deleted a comment from azure-pipelines bot Oct 30, 2024
PureWeen
PureWeen requested changes Oct 30, 2024
View reviewed changes
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.

rebase this for main please

@vishnumenon2684 vishnumenon2684 requested a review from a team as a code owner November 1, 2024 12:17
@vishnumenon2684 vishnumenon2684 changed the base branch from release/9.0.1xx to main November 1, 2024 12:18
@vishnumenon2684
Copy link
Contributor Author

rebase this for main please

Hi @PureWeen , I have rebased the PR to main.

@jfversluis
Copy link
Member

/rebase

@jfversluis
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jfversluis jfversluis requested a review from PureWeen November 1, 2024 19:01
@jsuarezruiz jsuarezruiz added area-navigation NavigationPage area-controls-shell Shell Navigation, Routes, Tabs, Flyout labels Nov 4, 2024
PureWeen
PureWeen requested changes Nov 6, 2024
View reviewed changes
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.

Can you add a test?

@Ahamed-Ali
Copy link
Contributor

Can you add a test?

I have added test cases @PureWeen

@PureWeen PureWeen removed this from the .NET 9 SR1 milestone Nov 7, 2024
@PureWeen PureWeen added this to the .NET 9 SR1.1 milestone Nov 7, 2024
@jsuarezruiz
Copy link
Contributor

/rebase

@dotnet dotnet deleted a comment from azure-pipelines bot Nov 12, 2024
@jsuarezruiz
Copy link
Contributor

/azp run

@rmarinho
Copy link
Member

/azp run

@dotnet dotnet deleted a comment from azure-pipelines bot Nov 18, 2024
@dotnet dotnet deleted a comment from azure-pipelines bot Nov 18, 2024
@PureWeen PureWeen self-assigned this Nov 21, 2024
@PureWeen PureWeen modified the milestones: .NET 9 SR1.1, .NET 9 SR2 Nov 22, 2024
@PureWeen PureWeen removed their assignment Nov 27, 2024
@PureWeen PureWeen added the p/0 Work that we can't release without label Nov 27, 2024
@rmarinho rmarinho merged commit 0893ce0 into dotnet:main Nov 27, 2024
108 checks passed
@sheiksyedm sheiksyedm added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Dec 6, 2024
@samhouts samhouts added fixed-in-9.0.21 fixed-in-net8.0-nightly This may be available in a nightly release! labels Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PureWeen PureWeen PureWeen requested changes

@rmarinho rmarinho rmarinho approved these changes

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

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

Assignees
No one assigned
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout area-navigation NavigationPage community ✨ Community Contribution fixed-in-9.0.21 fixed-in-net8.0-nightly This may be available in a nightly release! p/0 Work that we can't release without partner/syncfusion Issues / PR's with Syncfusion collaboration
Projects
MAUI SDK Ongoing
Status: Done
Milestone
.NET 9 SR2
Development

Successfully merging this pull request may close these issues.

[.NET 9] Shell Flyout menu not rendering after navigating from a MenuItem page
10 participants
@vishnumenon2684 @PureWeen @karthikraja-arumugam @jfversluis @Ahamed-Ali @jsuarezruiz @rmarinho @Redth @samhouts @sheiksyedm