Skip to content

Commit 719c33f

Browse files
ilonatommyCopilot
ilonatommy
and
Copilot
authored
Scroll in enhanced navigation behaves same as in client side routing (#60296)
* Fix: move the check before `location.href` change. * Fix: backwards/forwsrds button, preserve scroll position. * Fix: new page lands on the top + backwards/forwards lands on the saved scroll position. * Make the tests deterministic. --------- Co-authored-by: Copilot <[email protected]>
1 parent 18db463 commit 719c33f

File tree

11 files changed

+447
-11
lines changed

11 files changed

+447
-11
lines changed

‎src/Components/Web.JS/src/Rendering/Renderer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export function resetScrollAfterNextBatch(): void {
9595
shouldResetScrollAfterNextBatch = true;
9696
}
9797

98-
function resetScrollIfNeeded() {
98+
export function resetScrollIfNeeded() {
9999
if (shouldResetScrollAfterNextBatch) {
100100
shouldResetScrollAfterNextBatch = false;
101101

‎src/Components/Web.JS/src/Services/NavigationEnhancement.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import { synchronizeDomContent } from '../Rendering/DomMerging/DomSync';
55
import { attachProgrammaticEnhancedNavigationHandler, handleClickForNavigationInterception, hasInteractiveRouter, isForSamePath, isSamePageWithHash, notifyEnhancedNavigationListeners, performScrollToElementOnTheSamePage } from './NavigationUtils';
6+
import { resetScrollAfterNextBatch, resetScrollIfNeeded } from '../Rendering/Renderer';
67

78
/*
89
In effect, we have two separate client-side navigation mechanisms:
@@ -70,13 +71,19 @@ export function detachProgressivelyEnhancedNavigationListener() {
7071
window.removeEventListener('popstate', onPopState);
7172
}
7273

73-
function performProgrammaticEnhancedNavigation(absoluteInternalHref: string, replace: boolean) {
74+
function performProgrammaticEnhancedNavigation(absoluteInternalHref: string, replace: boolean) : void {
75+
const originalLocation = location.href;
76+
7477
if (replace) {
7578
history.replaceState(null, /* ignored title */ '', absoluteInternalHref);
7679
} else {
7780
history.pushState(null, /* ignored title */ '', absoluteInternalHref);
7881
}
7982

83+
if (!isForSamePath(absoluteInternalHref, originalLocation)) {
84+
resetScrollAfterNextBatch();
85+
}
86+
8087
performEnhancedPageLoad(absoluteInternalHref, /* interceptedLink */ false);
8188
}
8289

@@ -90,13 +97,20 @@ function onDocumentClick(event: MouseEvent) {
9097
}
9198

9299
handleClickForNavigationInterception(event, absoluteInternalHref => {
100+
const originalLocation = location.href;
101+
93102
const shouldScrollToHash = isSamePageWithHash(absoluteInternalHref);
94103
history.pushState(null, /* ignored title */ '', absoluteInternalHref);
95104

96105
if (shouldScrollToHash) {
97106
performScrollToElementOnTheSamePage(absoluteInternalHref);
98107
} else {
108+
let isSelfNavigation = isForSamePath(absoluteInternalHref, originalLocation);
99109
performEnhancedPageLoad(absoluteInternalHref, /* interceptedLink */ true);
110+
if (!isSelfNavigation) {
111+
resetScrollAfterNextBatch();
112+
resetScrollIfNeeded();
113+
}
100114
}
101115
});
102116
}
@@ -106,6 +120,7 @@ function onPopState(state: PopStateEvent) {
106120
return;
107121
}
108122

123+
// load the new page
109124
performEnhancedPageLoad(location.href, /* interceptedLink */ false);
110125
}
111126

‎src/Components/test/E2ETest/Infrastructure/WebDriverExtensions/WebDriverExtensions.cs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,70 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using OpenQA.Selenium;
5+
using OpenQA.Selenium.Support.UI;
6+
using System;
57

68
namespace Microsoft.AspNetCore.Components.E2ETest;
79

810
internal static class WebDriverExtensions
911
{
12+
private static string GetFindPositionScript(string elementId) =>
13+
$"return Math.round(document.getElementById('{elementId}').getBoundingClientRect().top + window.scrollY);";
14+
1015
public static void Navigate(this IWebDriver browser, Uri baseUri, string relativeUrl)
1116
{
1217
var absoluteUrl = new Uri(baseUri, relativeUrl);
1318

1419
browser.Navigate().GoToUrl("about:blank");
1520
browser.Navigate().GoToUrl(absoluteUrl);
1621
}
22+
23+
public static void WaitForElementToBeVisible(this IWebDriver browser, By by, int timeoutInSeconds = 5)
24+
{
25+
var wait = new DefaultWait<IWebDriver>(browser)
26+
{
27+
Timeout = TimeSpan.FromSeconds(timeoutInSeconds),
28+
PollingInterval = TimeSpan.FromMilliseconds(100)
29+
};
30+
wait.IgnoreExceptionTypes(typeof(NoSuchElementException), typeof(StaleElementReferenceException));
31+
wait.Until(driver =>
32+
{
33+
try
34+
{
35+
var element = driver.FindElement(by);
36+
return element.Displayed;
37+
}
38+
catch (StaleElementReferenceException)
39+
{
40+
// Retry finding the element
41+
return false;
42+
}
43+
});
44+
}
45+
46+
public static long GetElementPositionWithRetry(this IWebDriver browser, string elementId, int retryCount = 3, int delayBetweenRetriesMs = 100)
47+
{
48+
var jsExecutor = (IJavaScriptExecutor)browser;
49+
string script = GetFindPositionScript(elementId);
50+
browser.WaitForElementToBeVisible(By.Id(elementId));
51+
for (int i = 0; i < retryCount; i++)
52+
{
53+
try
54+
{
55+
var result = jsExecutor.ExecuteScript(script);
56+
if (result != null)
57+
{
58+
return (long)result;
59+
}
60+
}
61+
catch (OpenQA.Selenium.JavaScriptException)
62+
{
63+
// JavaScript execution failed, retry
64+
}
65+
66+
Thread.Sleep(delayBetweenRetriesMs);
67+
}
68+
69+
throw new Exception($"Failed to execute script after {retryCount} retries.");
70+
}
1771
}

‎src/Components/test/E2ETest/ServerRenderingTests/EnhancedNavigationTest.cs

Lines changed: 205 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures;
4+
using System.Threading.Tasks;
5+
using Components.TestServer.RazorComponents;
6+
using Microsoft.AspNetCore.Components.E2ETest;
57
using Microsoft.AspNetCore.Components.E2ETest.Infrastructure;
8+
using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures;
69
using Microsoft.AspNetCore.E2ETesting;
7-
using TestServer;
8-
using Xunit.Abstractions;
9-
using Components.TestServer.RazorComponents;
1010
using OpenQA.Selenium;
11+
using OpenQA.Selenium.BiDi.Communication;
1112
using OpenQA.Selenium.Support.Extensions;
13+
using TestServer;
14+
using Xunit.Abstractions;
15+
using Xunit.Sdk;
1216

1317
namespace Microsoft.AspNetCore.Components.E2ETests.ServerRenderingTests;
1418

@@ -662,6 +666,203 @@ public void CanUpdateHrefOnLinkTagWithIntegrity()
662666
Browser.Equal("rgba(0, 0, 255, 1)", () => originalH1Elem.GetCssValue("color"));
663667
}
664668

669+
[Theory]
670+
[InlineData(false, false, false)]
671+
[InlineData(false, true, false)]
672+
[InlineData(true, true, false)]
673+
[InlineData(true, false, false)]
674+
// [InlineData(false, false, true)] programmatic navigation doesn't work without enhanced navigation
675+
[InlineData(false, true, true)]
676+
[InlineData(true, true, true)]
677+
// [InlineData(true, false, true)] programmatic navigation doesn't work without enhanced navigation
678+
public void EnhancedNavigationScrollBehavesSameAsBrowserOnNavigation(bool enableStreaming, bool useEnhancedNavigation, bool programmaticNavigation)
679+
{
680+
// This test checks if the navigation to another path moves the scroll to the top of the page,
681+
// or to the beginning of a fragment, regardless of the previous scroll position
682+
string landingPageSuffix = enableStreaming ? "" : "-no-streaming";
683+
string buttonKeyword = programmaticNavigation ? "-programmatic" : "";
684+
Navigate($"{ServerPathBase}/nav/scroll-test{landingPageSuffix}");
685+
EnhancedNavigationTestUtil.SuppressEnhancedNavigation(this, shouldSuppress: !useEnhancedNavigation, skipNavigation: true);
686+
687+
// "landing" page: scroll maximally down and go to "next" page - we should land at the top of that page
688+
AssertWeAreOnLandingPage();
689+
690+
// staleness check is used to assert enhanced navigation is enabled/disabled, as requested
691+
var elementForStalenessCheckOnNextPage = Browser.Exists(By.TagName("html"));
692+
693+
var button1Id = $"do{buttonKeyword}-navigation";
694+
var button1Pos = Browser.GetElementPositionWithRetry(button1Id);
695+
Browser.SetScrollY(button1Pos);
696+
Browser.Exists(By.Id(button1Id)).Click();
697+
698+
// "next" page: check if we landed at 0, then navigate to "landing"
699+
AssertWeAreOnNextPage();
700+
WaitStreamingRendersFullPage(enableStreaming);
701+
string fragmentId = "some-content";
702+
Browser.WaitForElementToBeVisible(By.Id(fragmentId));
703+
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnNextPage);
704+
Assert.Equal(0, Browser.GetScrollY());
705+
var elementForStalenessCheckOnLandingPage = Browser.Exists(By.TagName("html"));
706+
var fragmentScrollPosition = Browser.GetElementPositionWithRetry(fragmentId);
707+
Browser.Exists(By.Id(button1Id)).Click();
708+
709+
// "landing" page: navigate to a fragment on another page - we should land at the beginning of the fragment
710+
AssertWeAreOnLandingPage();
711+
WaitStreamingRendersFullPage(enableStreaming);
712+
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnLandingPage);
713+
714+
var button2Id = $"do{buttonKeyword}-navigation-with-fragment";
715+
Browser.Exists(By.Id(button2Id)).Click();
716+
AssertWeAreOnNextPage();
717+
WaitStreamingRendersFullPage(enableStreaming);
718+
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnNextPage);
719+
var expectedFragmentScrollPosition = fragmentScrollPosition - 1;
720+
Assert.Equal(expectedFragmentScrollPosition, Browser.GetScrollY());
721+
}
722+
723+
[Theory]
724+
[InlineData(false, false, false)]
725+
[InlineData(false, true, false)]
726+
[InlineData(true, true, false)]
727+
[InlineData(true, false, false)]
728+
// [InlineData(false, false, true)] programmatic navigation doesn't work without enhanced navigation
729+
[InlineData(false, true, true)]
730+
[InlineData(true, true, true)]
731+
// [InlineData(true, false, true)] programmatic navigation doesn't work without enhanced navigation
732+
public void EnhancedNavigationScrollBehavesSameAsBrowserOnBackwardsForwardsAction(bool enableStreaming, bool useEnhancedNavigation, bool programmaticNavigation)
733+
{
734+
// This test checks if the scroll position is preserved after backwards/forwards action
735+
string landingPageSuffix = enableStreaming ? "" : "-no-streaming";
736+
string buttonKeyword = programmaticNavigation ? "-programmatic" : "";
737+
Navigate($"{ServerPathBase}/nav/scroll-test{landingPageSuffix}");
738+
EnhancedNavigationTestUtil.SuppressEnhancedNavigation(this, shouldSuppress: !useEnhancedNavigation, skipNavigation: true);
739+
740+
// "landing" page: scroll to pos1, navigate away
741+
AssertWeAreOnLandingPage();
742+
WaitStreamingRendersFullPage(enableStreaming);
743+
744+
// staleness check is used to assert enhanced navigation is enabled/disabled, as requested
745+
var elementForStalenessCheckOnNextPage = Browser.Exists(By.TagName("html"));
746+
747+
var buttonId = $"do{buttonKeyword}-navigation";
748+
Browser.WaitForElementToBeVisible(By.Id(buttonId));
749+
var landingPagePos1 = Browser.GetElementPositionWithRetry(buttonId) - 100;
750+
Browser.SetScrollY(landingPagePos1);
751+
Browser.Exists(By.Id(buttonId)).Click();
752+
753+
// "next" page: scroll to pos1, navigate away
754+
AssertWeAreOnNextPage();
755+
WaitStreamingRendersFullPage(enableStreaming);
756+
Browser.WaitForElementToBeVisible(By.Id(buttonId));
757+
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnNextPage);
758+
var elementForStalenessCheckOnLandingPage = Browser.Exists(By.TagName("html"));
759+
var nextPagePos1 = Browser.GetElementPositionWithRetry(buttonId) - 100;
760+
// make sure we are expecting different scroll positions on the 1st and the 2nd page
761+
Assert.NotEqual(landingPagePos1, nextPagePos1);
762+
Browser.SetScrollY(nextPagePos1);
763+
Browser.Exists(By.Id(buttonId)).Click();
764+
765+
// "landing" page: scroll to pos2, go backwards
766+
AssertWeAreOnLandingPage();
767+
WaitStreamingRendersFullPage(enableStreaming);
768+
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnLandingPage);
769+
var landingPagePos2 = 500;
770+
Browser.SetScrollY(landingPagePos2);
771+
Browser.Navigate().Back();
772+
773+
// "next" page: check if we landed on pos1, move the scroll to pos2, go backwards
774+
AssertWeAreOnNextPage();
775+
WaitStreamingRendersFullPage(enableStreaming);
776+
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnNextPage);
777+
AssertScrollPositionCorrect(useEnhancedNavigation, nextPagePos1);
778+
779+
var nextPagePos2 = 600;
780+
Browser.SetScrollY(nextPagePos2);
781+
Browser.Navigate().Back();
782+
783+
// "landing" page: check if we landed on pos1, move the scroll to pos3, go forwards
784+
AssertWeAreOnLandingPage();
785+
WaitStreamingRendersFullPage(enableStreaming);
786+
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnLandingPage);
787+
AssertScrollPositionCorrect(useEnhancedNavigation, landingPagePos1);
788+
var landingPagePos3 = 700;
789+
Browser.SetScrollY(landingPagePos3);
790+
Browser.Navigate().Forward();
791+
792+
// "next" page: check if we landed on pos1, go forwards
793+
AssertWeAreOnNextPage();
794+
WaitStreamingRendersFullPage(enableStreaming);
795+
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnNextPage);
796+
AssertScrollPositionCorrect(useEnhancedNavigation, nextPagePos2);
797+
Browser.Navigate().Forward();
798+
799+
// "scroll" page: check if we landed on pos2
800+
AssertWeAreOnLandingPage();
801+
WaitStreamingRendersFullPage(enableStreaming);
802+
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnLandingPage);
803+
AssertScrollPositionCorrect(useEnhancedNavigation, landingPagePos2);
804+
}
805+
806+
private void AssertScrollPositionCorrect(bool useEnhancedNavigation, long previousScrollPosition)
807+
{
808+
// from some reason, scroll position sometimes differs by 1 pixel between enhanced and browser's navigation
809+
// browser's navigation is not precisely going backwards/forwards to the previous state
810+
var currentScrollPosition = Browser.GetScrollY();
811+
string messagePart = useEnhancedNavigation ? $"{previousScrollPosition}" : $"{previousScrollPosition} or {previousScrollPosition - 1}";
812+
bool isPreciselyWhereItWasLeft = currentScrollPosition == previousScrollPosition;
813+
bool isPixelLowerThanItWasLeft = currentScrollPosition == (previousScrollPosition - 1);
814+
bool success = useEnhancedNavigation
815+
? isPreciselyWhereItWasLeft
816+
: (isPreciselyWhereItWasLeft || isPixelLowerThanItWasLeft);
817+
Assert.True(success, $"The expected scroll position was {messagePart}, but it was found at {currentScrollPosition}.");
818+
}
819+
820+
private void AssertEnhancedNavigation(bool useEnhancedNavigation, IWebElement elementForStalenessCheck, int retryCount = 3, int delayBetweenRetriesMs = 100)
821+
{
822+
bool enhancedNavigationDetected = false;
823+
for (int i = 0; i < retryCount; i++)
824+
{
825+
try
826+
{
827+
enhancedNavigationDetected = !IsElementStale(elementForStalenessCheck);
828+
Assert.Equal(useEnhancedNavigation, enhancedNavigationDetected);
829+
return;
830+
}
831+
catch (XunitException)
832+
{
833+
// Maybe the check was done too early to change the DOM ref, retry
834+
}
835+
836+
Thread.Sleep(delayBetweenRetriesMs);
837+
}
838+
string expectedNavigation = useEnhancedNavigation ? "enhanced navigation" : "browser navigation";
839+
string isStale = enhancedNavigationDetected ? "is not stale" : "is stale";
840+
var isNavigationSupressed = (string)((IJavaScriptExecutor)Browser).ExecuteScript("return sessionStorage.getItem('suppress-enhanced-navigation');");
841+
throw new Exception($"Expected to use {expectedNavigation} because 'suppress-enhanced-navigation' is set to {isNavigationSupressed} but the element from previous path {isStale}");
842+
}
843+
844+
private void AssertWeAreOnLandingPage()
845+
{
846+
string infoName = "test-info-1";
847+
Browser.WaitForElementToBeVisible(By.Id(infoName), timeoutInSeconds: 20);
848+
Browser.Equal("Scroll tests landing page", () => Browser.Exists(By.Id(infoName)).Text);
849+
}
850+
851+
private void AssertWeAreOnNextPage()
852+
{
853+
string infoName = "test-info-2";
854+
Browser.WaitForElementToBeVisible(By.Id(infoName), timeoutInSeconds: 20);
855+
Browser.Equal("Scroll tests next page", () => Browser.Exists(By.Id(infoName)).Text);
856+
}
857+
858+
private void WaitStreamingRendersFullPage(bool enableStreaming)
859+
{
860+
if (enableStreaming)
861+
{
862+
Browser.WaitForElementToBeVisible(By.Id("some-content"));
863+
}
864+
}
865+
665866
private void AssertEnhancedUpdateCountEquals(long count)
666867
=> Browser.Equal(count, () => ((IJavaScriptExecutor)Browser).ExecuteScript("return window.enhancedPageUpdateCount;"));
667868

‎src/Components/test/E2ETest/ServerRenderingTests/EnhancedNavigationTestUtil.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,7 @@ public static void SuppressEnhancedNavigation<TServerFixture>(ServerTestBase<TSe
3232

3333
public static long GetScrollY(this IWebDriver browser)
3434
=> Convert.ToInt64(((IJavaScriptExecutor)browser).ExecuteScript("return window.scrollY"), CultureInfo.CurrentCulture);
35+
36+
public static long SetScrollY(this IWebDriver browser, long value)
37+
=> Convert.ToInt64(((IJavaScriptExecutor)browser).ExecuteScript($"window.scrollTo(0, {value})"), CultureInfo.CurrentCulture);
3538
}

0 commit comments

Comments
 (0)