dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.37k stars 9.99k forks source link

Blazor NavigationManager.NavigateTo always scrolls page to the top. #40190

Open groogiam opened 2 years ago

groogiam commented 2 years ago

Is there an existing issue for this?

Describe the bug

It appears the navigation manger in .NET 6 always scrolls the page to the top even if NavigateTo call is just updating the query string.

Expected Behavior

Navigations that do not change the page should not reset the viewport position.

Steps To Reproduce

Will post a repro shortly.

Exceptions (if any)

No response

.NET Version

6.0.102

Anything else?

No response

groogiam commented 2 years ago

Repro

https://github.com/groogiam/AspNetCore40190Repro

rogihee commented 2 years ago

.NET6 has a new API to manipulate the query string NavigationManager.GetUriWithQueryParameter, see the docs here https://docs.microsoft.com/en-us/aspnet/core/blazor/fundamentals/routing?view=aspnetcore-6.0#query-strings

Did you try that?

groogiam commented 2 years ago

@rogihee Thanks for the tip. I have tried this. That api does not actually navigate to anything it just gets a new value for the querystring. This does not really solve the issue as I still have to navigate to that uri to update the browser url and the NavigateTo call is what is scrolling the viewport needlessly.

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.

lukblazewicz commented 2 years ago

Just to bump up - this behaviour affects our application as well (quite significantly). Is there any workaround for that until the issue gets resolved?

groogiam commented 2 years ago

@lukblazewicz I have had some success using JS interop to capture the current page scroll position and and then trying to reset the position after the update query string operation completes. I had to add some random Task.Delay statements in my C# code to get things to work and the user experience is still pretty bad as the page jumps up and then back down but at least your users don't have to scroll manually back to where they were. Hopefully this helps.

groogiam commented 2 years ago

@mkArtakMSFT I'm not sure why this is tagged as an enhancement, I'm fairly certain this it is a regression from Blazor on .NET 5. The issue did not manifest until I upgraded from .NET 5 to .NET 6. This regression really impacts user experience of this product.

lukblazewicz commented 2 years ago

@groogiam Thanks for the tip, we will have to consider it if there is little hope to get this bug fixed soon. @mkArtakMSFT Is there any chance this issue gets promoted to be fixed in the upcoming releases?

dp-sgr commented 2 years ago

@groogiam Sounds like #36605

Look at the Workaround from @smirceamihai:

https://github.com/dotnet/aspnetcore/pull/12423#issuecomment-778774206

groogiam commented 2 years ago

@dp-sgr Disregard my last post. This does work. There is some code in a third party component libraries that scrolls a different element to top. This solution does work for the window scroll. Thanks again for pointing this work around out.

pheuter commented 2 years ago

This is a non-trivial blocker for me. Imagine the common filter experience where facets are stored in the URL as query strings. Whenever you'd call NavManager.NavigateTo(NavManger.GetUriWithQueryParameter("color", "blue")) the page will scroll up and disrupt whatever view the user was filtering on.

We're using Blazor Server, and afaik there's no way to manually keep NavManager in sync with a workaround that uses js pushState() or replaceState(). Being able to intentionally opt-out of scrolling to the top seems important.

I found this bit in the source code, being able to conditionally call resetScrollAfterNextBatch() seems like the desirable outcome.

function performInternalNavigation(absoluteInternalHref: string, interceptedLink: boolean, replace: boolean) {
  // Since this was *not* triggered by a back/forward gesture (that goes through a different
  // code path starting with a popstate event), we don't want to preserve the current scroll
  // position, so reset it.
  // To avoid ugly flickering effects, we don't want to change the scroll position until
  // we render the new page. As a best approximation, wait until the next batch.
  resetScrollAfterNextBatch();

  if (!replace) {
    history.pushState(null, /* ignored title */ '', absoluteInternalHref);
  } else {
    history.replaceState(null, /* ignored title */ '', absoluteInternalHref);
  }

  notifyLocationChanged(interceptedLink);
}

/cc @SteveSandersonMS I noticed you introduced this scrolling functionality in https://github.com/dotnet/aspnetcore/pull/12423, perhaps you may have more relevant insight into the issue and possible workarounds. I appreciate any feedback.

kyletinsley commented 2 years ago

Auto-scrolling to top is causing issues for me as well, particularly in mobile layouts where screen is more narrow. I attempted to use JS interop for window.history, but that has side-effects w/ NaviationManager.LocationChanged and OnParameterSet[Async]. A scrollToTop optional parameter on NavigateTo would be excellent!

kyletinsley commented 2 years ago

This appears to be causing issues for third-party component libraries as well: https://forum.radzen.com/t/prevent-radzenbody-from-scrolling-to-top-on-navigation-that-only-changes-query-string/10783

gregsdennis commented 2 years ago

Yes, also affecting my WASM app which uses anchor navigation in headers. When the user clicks on a header link (which only adds an anchor fragment, e.g. #foo, to the URL), the page scrolls all the way to the top.

dennml commented 2 years ago

This creates an unpleasant experience. We really need the ability to change the address using NavigationManager.NavigateTo Without scrolling to the top of the page every time. Does anyone know if this will get attention?

SteveSandersonMS commented 2 years ago

.NET 7 has already closed for new features, but I can see this is a common requirement so it's definitely something we can consider for .NET 8.

In the meantime, as a workaround, you could implement manual control over this using JavaScript. For example,

<!-- In index.html -->
<script>
    const origScrollTo = window.scrollTo;
    window.scrollTo = (x, y) => {
        const shouldSkip = true; // TODO: Insert your own logic here
        if (x === 0 && y === 0 && shouldSkip)
            return;
        return origScrollTo.apply(this, arguments);
    };
</script>

You'd have to put in some logic of your own to control the shouldSkip variable, deciding whether or not any given call should be skipped. For example you could have the .NET side use JS interop to set some value for the variable before each navigation call, and reset it inside the logic here.

I know it's not very convenient so I do agree we should offer built-in control for this!

ghost commented 2 years ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

TanayParikh commented 1 year ago

Related to https://github.com/dotnet/aspnetcore/issues/8393 ?

ledpup commented 1 year ago

The code above is slightly wrong. "Arguments" will be undefined.

"The arguments object is a local variable available within all non-arrow functions."

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/arguments

Use this code instead. I.e., replace arguments with [x, y]

<script>
      var skipScrollTo = false;
      const origScrollTo = window.scrollTo;
      window.scrollTo = (x, y) => {
        if (x === 0 && y === 0 && skipScrollTo) {
          skipScrollTo = false;
          return;
        }
        return origScrollTo.apply(this, [x, y]);
      };

      function willSkipScrollTo(newValue) {
        skipScrollTo = newValue;
      }
</script>
baktay commented 1 year ago

Please make this easy in .NET 8 for Blazor Server like the .net Framework page code alternatively Page.MaintainScrollPositionOnPostBack = true ; line in codebehind. Something similar would be great.

Also, HTML feature anchor navigation does not work in blazor server. For example uri https://example.com/#footer scrolls to #example point on page. It even works by default HTML to point to a particular anchor in another page via link url by adding .../#footer. This is very useful and I am struggling to update previous projects that made use of this heavily. Please make it work in razor pages in Blazor Server in .NET 8

ghost commented 10 months ago

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

KevinH404 commented 9 months ago

@DerekFoulk we workaround this by adding window.scrollTo = undefined; in our main layout.

DerekFoulk commented 9 months ago

@KevinH404 Thanks for the tip! In my case- I'm just an idiot... I was using <a href="#"...> for my buttons. It was scrolling to the top of the page because the URL was changed. I removed the href attribute from the anchor tag and "voila!". No more unwanted scrolling.

AndrewTriesToCode commented 5 months ago

Saw this while using static SSR with streaming... solved with this note from the docs:

<script src="/_framework/blazor.web.js" autostart="false"></script>
<script>
    Blazor.start({
        ssr: { disableDomPreservation: true }
    });
</script>

Obviously won't help if you are doing anything interactive with NavigationManager...

sonnemaf commented 5 months ago

I just found this blog in which it is explained how to disable the enhanced navigation. Globally using the solution @AndrewTriesToCode mention but also locally.

The following examples disable enhanced navigation using the data-enhance-nav attribute. Below, data-enhance-nav="false" disables the feature for a single link instance.

<a href="redirect" data-enhance-nav="false">
    GET without enhanced navigation
</a>
AndrewTriesToCode commented 5 months ago

Nice find. @sonnemaf

I have to admit for SSR I did NOT expect that enhanced navigation would be in play at all especially for local anchors.