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

[Blazor] Navigating to a page will scroll to top before rendering new page #53863

Open
1 task done
danielchalmers opened this issue Feb 7, 2024 · 10 comments
Open
1 task done
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. feature-blazor-hybrid feature-blazor-navigation
Milestone

Comments

@danielchalmers
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

Using NavigationManager.NavigateTo to change pages while you are partially scrolled down the page will visibly and jarringly scroll to the top before navigating to the new page.

Expected Behavior

The user should keep seeing the same viewport until the new page has rendered.

I know there's going to be a technical reason for this, but it just looks so unpleasant and I don't know how to work around it.

Steps To Reproduce

This was done on .NET MAUI Blazor Hybrid with MudBlazor. If a repo is needed, please let me know and I'll happily go make one.

Exceptions (if any)

No response

.NET Version

8.0.100

Anything else?

You can see in this video that I scroll down then click a button to navigate the page which then scrolls back up before navigating to the new page.

Video.mp4

It looks really bad for the user.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label Feb 7, 2024
danielchalmers added a commit to danielchalmers/JournalApp that referenced this issue Feb 7, 2024
Wasn't working 100% and I need to fix scrolling in general dotnet/aspnetcore#53863

This reverts commit 9b92b20.
@SteveSandersonMS
Copy link
Member

If a repo is needed, please let me know and I'll happily go make one.

Thanks, yes, I think we will need a repro (as a public GitHub repo) because that shouldn't happen by default. Please make sure the repro is minimal and doesn't use MudBlazor or other external libraries, so we can see it's a problem with Blazor itself.

@danielchalmers
Copy link
Contributor Author

If a repo is needed, please let me know and I'll happily go make one.

Thanks, yes, I think we will need a repro (as a public GitHub repo) because that shouldn't happen by default. Please make sure the repro is minimal and doesn't use MudBlazor or other external libraries, so we can see it's a problem with Blazor itself.

@SteveSandersonMS

Hi, it's occurring before the OnInitializedAsync event.

Video.mp4

(it pauses because it hits the breakpoint)

I must be doing a bit more work in my app because it's much harder to see on a fresh template without the breakpoint.

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Feb 8, 2024

Thanks, that does explain it. I think this is a behavior that has always existed in Blazor since the beginning, though it requires a particular combination of things to happen in order to produce the UX weirdness you are observing.

Minimal repro

In an app with global interactivity:

  1. In page A which is big enough to be scrollable, add a button at the bottom that does programmatic navigation: <button @onclick="@(() => NavigationManager.NavigateTo("b"))">Do nav</button>
  2. In page B, cause it to block the CPU for a while on startup (e.g., in OnInitialized, add Thread.Sleep(1000)).
  3. Run the app and click the button in page A. Observe it scrolls to the top before page B is rendered.

Explanation

It's desirable to navigate to the top when the user moves to a new page. However, the client-side code doesn't really know which render corresponds to a "new page", so it uses the following heuristic:

  • After triggering a navigation and completing any async OnNavigating flow, client-side code sets a flag saying we want to scroll to the top on the next render
  • On each render, client-side code checks if that flag is set, and if so, scrolls to the top and clears the flag

This works correctly as long as, after each navigation is triggered, the next render is the one for the new page. And that's always the case given that navigation occurs synchronously between those two steps above, and that the new page always renders synchronously. It does not work if:

  • the new page does CPU-blocking work that delays its first render long enough to be observable by humans
  • and we're running outside the browser's JS event loop (e.g., on Server or WebView, but not WebAssembly), so the browser can update the UI even though the Blazor code is blocked

Workaround

You can work around this by suppressing the rendering of the old page when you know you're navigating away from it, e.g.:

<button @onclick="@DoNavigation">Do nav</button>

@code {
    bool suppressRender;

    protected override bool ShouldRender()
        => !suppressRender;

    private void DoNavigation()
    {
        suppressRender = true;
        NavigationManager.NavigateTo("/");
    }
}

Longer-term solution

We could consider having "scroll to top" be triggered by something inside Router that happens only after it knows it's completed a render after changing the current page component. There may be lots of subtleties that make this difficult, though.

@danielchalmers
Copy link
Contributor Author

@SteveSandersonMS

Really appreciate the detailed the response. Weird thing is that the workaround works on my sample project but not on the production app. Even protected override bool ShouldRender() => false; seems to do nothing at all. Do you know what could cause that? I don't inherit from anything.

@SteveSandersonMS
Copy link
Member

protected override bool ShouldRender() => false; will stop all except the first render from your component. I'm not sure of any reason why it could fail to have an effect.

However it's possible that your real-world app triggers other component renders besides the one you've suppressed rendering (i.e., in between when you do the navigation and when the new page renders itself). That would prevent the workaround from working.

@danielchalmers
Copy link
Contributor Author

danielchalmers commented Feb 8, 2024

For future reference: The culprit was the MudBlazor button which probably triggers a render at the same time it calls OnClick->Navigate because you can see it scroll to the top even with ShouldRender() => false. You do not see the same with a regular <button>.

MudBlazor/MudBlazor#8151

@mkArtakMSFT mkArtakMSFT added bug This issue describes a behavior which is not expected - a bug. feature-blazor-navigation labels Feb 14, 2024
@mkArtakMSFT mkArtakMSFT added this to the Backlog milestone Feb 14, 2024
@Geccoka
Copy link

Geccoka commented Feb 19, 2024

I'm also using MudBlazor and had this issue too.

I've figured out a workaround that works in my application in a .NET7 Blazor Server app. For some reason that I don't understand, after I've put a NavigationLock with an empty OnBeforeInternalNavigation handler, the page only scrolled up after the new page has rendered, here is the solution.

Relevant part of App.razor:

<Router AppAssembly="@typeof(App).Assembly">
    <Found Context="routeData">
        ...
        <NavigationLock OnBeforeInternalNavigation="@EventCallback.Empty" />
    </Found>
    ...
</Router>

I've modified nothing else in my app.

@danielchalmers
Copy link
Contributor Author

@Geccoka

Weird, I'm not getting the same result on MAUI Hybrid 8.0:

<Router AppAssembly="@typeof(MauiProgram).Assembly">
	<Found Context="routeData">
		<RouteView RouteData="@routeData" DefaultLayout="@typeof(MainLayout)" />
		@*<FocusOnNavigate RouteData="@routeData" Selector="h1" />*@

		@* https://github.com/dotnet/aspnetcore/issues/53863#issuecomment-1952669759 *@
		<NavigationLock OnBeforeInternalNavigation="@EventCallback.Empty" />
	</Found>
</Router>

or Blazor Server 7.0

<Router AppAssembly="@typeof(App).Assembly">
    <Found Context="routeData">
        <RouteView RouteData="@routeData" DefaultLayout="@typeof(MainLayout)" />
	@* https://github.com/dotnet/aspnetcore/issues/53863#issuecomment-1952669759 *@
	<NavigationLock OnBeforeInternalNavigation="@EventCallback.Empty" />
    </Found>
    <NotFound>
        <PageTitle>Not found</PageTitle>
        <LayoutView Layout="@typeof(MainLayout)">
            <p role="alert">Sorry, there's nothing at this address.</p>
        </LayoutView>
    </NotFound>
</Router>

Are you using NavigationManager.NavigateTo?

@Geccoka
Copy link

Geccoka commented Feb 20, 2024

Yes, I'm using NavigationManager.NavigateTo, but after further testing I must say this workaround only solves the issue in certain cases. There are places in my app where this doesn't work :(. But putting an await Task.Delay(100) to the OnBeforeInternalNavigation handler solves it :). Ofc this will delay every navigation by 100ms.

danielchalmers added a commit to danielchalmers/JournalApp that referenced this issue Mar 5, 2024
Issue described in dotnet/aspnetcore#53863 and for MudBlazor at MudBlazor/MudBlazor#8151.

The MudBlazor part was remedied for MudButton with MudBlazor/MudBlazor#8203 in 6.17.0 but MudIconButton and MudMenuItem were not impacted and are awaiting MudBlazor/MudBlazor#8281
danielchalmers added a commit to danielchalmers/JournalApp that referenced this issue Mar 7, 2024
Issue described in dotnet/aspnetcore#53863 and for MudBlazor at MudBlazor/MudBlazor#8151.

The MudBlazor part was remedied for MudButton with MudBlazor/MudBlazor#8203 in 6.17.0 but MudIconButton and MudMenuItem were not impacted and are awaiting MudBlazor/MudBlazor#8281
@hyigit-adelcon
Copy link

As a temporary solution you can add this to your javascript file (or MainLayout):

window.scrollTo = undefined;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. feature-blazor-hybrid feature-blazor-navigation
Projects
None yet
Development

No branches or pull requests

5 participants