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.

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

Fixed Shell Navigating event issue when switching tabs #25749

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Vignesh-SF3580
Copy link
Contributor

@Vignesh-SF3580 Vignesh-SF3580 commented Nov 8, 2024

Issue Detail

Clicking on an already-selected tab triggered the Navigating event unnecessarily.

Root Cause

The event was triggered without checking if the tab was already selected.

Description of Change

Modified the code to trigger Navigating only if a new tab is selected.

Tested the behaviour in the following platforms

  • Android
  • Windows
  • iOS
  • Mac

Issues Fixed

Fixes #25599

Screenshots

Before Issue Fix After Issue Fix
BeforeFix.mov
AfterFix.mov

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Nov 8, 2024
Copy link
Contributor

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

@jsuarezruiz jsuarezruiz added the area-controls-shell Shell Navigation, Routes, Tabs, Flyout label Nov 11, 2024
@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@Vignesh-SF3580 Vignesh-SF3580 marked this pull request as ready for review November 15, 2024 10:56
@Vignesh-SF3580 Vignesh-SF3580 requested a review from a team as a code owner November 15, 2024 10:56
@jsuarezruiz
Copy link
Contributor

/azp run

@rmarinho
Copy link
Member

/rebase

@rmarinho
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@@ -0,0 +1,27 @@
#if ANDROID // Facing a full exception when clicking on the already selected tab(App.Tap()) in Windows, iOS, and macOS, so excluded those platforms.
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 share the exception? Want to determinate if is related with a possible bug on Android or if we need to review something at testing stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have revalidated the test cases once more with the latest source and I can confirm that it is now possible to click on the already selected tab using the tab title on Windows and AutomationId on iOS and Mac.

I have modified the test cases to run successfully in all platforms. Could you please review and let me know if you have any concerns?

@jsuarezruiz
Copy link
Contributor

/rebase

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@sheiksyedm sheiksyedm added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Dec 6, 2024
@PureWeen PureWeen self-assigned this Dec 12, 2024
@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Foda
Foda previously approved these changes Jan 6, 2025
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.

  1. Can you split these changes into two PRs?
  2. I think the implementation on iOS isn't addressing the issue. The problem the user is reporting is that the navigating event isn't firing when they reclick on the Tab. Ideally we don't disable this behavior but we instead route it through the Shell.Navigating features so that users can enable/disable this behavior

@Vignesh-SF3580
Copy link
Contributor Author

  1. Can you split these changes into two PRs?
  2. I think the implementation on iOS isn't addressing the issue. The problem the user is reporting is that the navigating event isn't firing when they reclick on the Tab. Ideally we don't disable this behavior but we instead route it through the Shell.Navigating features so that users can enable/disable this behavior
  1. I have divided the changes into two separate PRs: 25749 and 27197.
  2. Regarding the reported issue: The user highlighted that clicking on an already selected tab triggers the  Navigating event with the same navigation parameters. This behavior is inconsistent across platforms.

Current Behavior:

  • iOS and macOS: Clicking on an already selected tab triggers both the Navigating and Navigated events, and navigation occurs.
  • Android: Clicking on an already selected tab does not trigger the Navigating or Navigated events, and no navigation occurs.
  • Windows: Switching between tabs does not trigger the Navigating or Navigated events. To address this, I have implemented logic on Windows to ensure these events are triggered. (27197)

Behavior After the Fix:

  • Clicking on an already selected tab will no longer trigger the Navigating event, and no navigation will occur.
  • The Navigating event will only be triggered when switching to a different (unselected) tab.

Example Scenario:
Consider a tabbed application with three tabs: Home, Profile, and Settings.

  • When the user selects the Home tab, the Navigating event is triggered, and the application navigates to the default Home view.
  • From the Home view, the user navigates to another page (e.g., AnotherPage) by clicking a button. If the user then clicks the Home tab again (which is already selected), the Navigating event will not be triggered, and no navigation will occur. Returning to the previous page can only be done using the back navigation button.
  • However, if the user switches to the Profile tab, the Navigating event will be triggered, and the application will navigate to the Profile view.
     
    Note: I implemented this based on the behavior observed in Android.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration
Projects
Status: Changes Requested
Development

Successfully merging this pull request may close these issues.

OnNavigating wrong target when tapping the same tab
6 participants