FocusOnNavigate Component does not work for routes with different parameter values

mtavares628 commented 2 years ago

Describe the bug

When using the FocusOnNavigate component where nav links point to the same component, but with different parameter values, the focus is not set to the h1 header of the content if the user is already on that page, but with a different parameter value.

To Reproduce

https://github.com/mtavares628/FocusOnNav.git

Click on Counter, and you will see that the header's focus is set.
Click on Client 1 and the header's focus is again set.
Click on Client 2 and the focus stays on the Nav Link.

I believe this is because the FocusOnNavigate component is just checking the PageType of the component to see if it is the same as the last selected PageType, and not the full route with its parameters. I would expect that if the full route is different, then it should still set focus to the h1 header.

Exceptions (if any)

No Exception is generated

Further technical details

ASP.NET Core version: 6.0
The IDE (VS / VS Code/ VS4Mac) you're running on, and its version: VS 2022 64 bit 17.0.1
Include the output of dotnet --info:

dotnet --info Output

``` .NET SDK (reflecting any global.json): Version: 6.0.100 Commit: 9e8b04bbff Runtime Environment: OS Name: Windows OS Version: 10.0.19044 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\6.0.100\ Host (useful for support): Version: 6.0.0 Commit: 4822e3c3aa .NET SDKs installed: 1.0.4 [C:\Program Files\dotnet\sdk] 2.1.4 [C:\Program Files\dotnet\sdk] 2.1.202 [C:\Program Files\dotnet\sdk] 2.1.302 [C:\Program Files\dotnet\sdk] 2.1.526 [C:\Program Files\dotnet\sdk] 2.1.617 [C:\Program Files\dotnet\sdk] 3.1.414 [C:\Program Files\dotnet\sdk] 5.0.103 [C:\Program Files\dotnet\sdk] 5.0.303 [C:\Program Files\dotnet\sdk] 5.0.401 [C:\Program Files\dotnet\sdk] 5.0.402 [C:\Program Files\dotnet\sdk] 6.0.100 [C:\Program Files\dotnet\sdk] .NET runtimes installed: Microsoft.AspNetCore.All 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.24 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.24 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.19 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.20 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.10 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 1.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 1.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.24 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.19 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.20 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 5.0.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 5.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 5.0.10 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 5.0.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 5.0.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 3.1.19 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 3.1.20 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 5.0.3 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 5.0.9 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 5.0.10 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 5.0.11 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 5.0.12 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] ```

TanayParikh commented 2 years ago

Thanks for reporting this @mtavares628, I checked out your repro and I see what you mean.

I believe this is because the FocusOnNavigate component is just checking the PageType of the component to see if it is the same as the last selected PageType, and not the full route with its parameters

That's exactly what's happening:

https://github.com/dotnet/aspnetcore/blob/28ecab610cce3afd17305b3929ba5c39c63ef02d/src/Components/Web/src/Routing/FocusOnNavigate.cs#L48

I would expect that if the full route is different, then it should still set focus to the h1 header.

I think that's a logical assumption. Will confirm that's the desired behavior (or document if it isn't).

ghost commented 2 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

TanayParikh commented 2 years ago

Discussed offline, we may want to make this an optional functionality. You may be able to derive from the existing FocusOnNavigate to achieve this yourself.

mtavares628 commented 2 years ago

Can you elaborate on why this would be optional, since it doesn't function as intended consistently? Also, can you explain the workaround deriving from the existing control? Is there a way to get the full URL from the routeData to check that, or do I need utilize a different method?

TanayParikh commented 2 years ago

Can you elaborate on why this would be optional, since it doesn't function as intended consistently?

Depending on the use case, you may want to be parameter agnostic, and only trigger for when you navigate between actual pages. Doing it based on the full route could be considered a breaking change.

Also, can you explain the workaround deriving from the existing control? Is there a way to get the full URL from the routeData to check that, or do I need utilize a different method?

You should be able to derive from the existing FocusOnNavigate and then override the OnParametersSet to consider the full RouteData.RouteValues.

symbiotic commented 1 year ago

I have the same problem.

You should be able to derive from the existing FocusOnNavigate and then override the OnParametersSet to consider the full RouteData.RouteValues.

That doesn't seem to work because FocusOnNavigate.OnParametersSet sets the private members _lastNavigatedPageType and _focusAfterRender, which are inaccessible from a derived class.

FocusOnNavigate also uses DomWrapperInterop.FocusBySelector which is also inaccessible. Incase you were thinking of copying the entire file and modifying it.

dotnet / aspnetcore