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

Scroll in enhanced navigation behaves same as in client side routing #60296

Merged
merged 26 commits into from
Feb 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
d10ebfa
Fix.
ilonatommy Feb 10, 2025
06655d5
Fix: move the check before `location.href` change.
ilonatommy Feb 10, 2025
8da794c
Fix: backwards/forwsrds button, preserve scroll position.
ilonatommy Feb 10, 2025
97420c2
Draft of tests.
ilonatommy Feb 11, 2025
cfe8046
Fix: new page lands on the top + backwards/forwards lands on the save…
ilonatommy Feb 17, 2025
d56bc30
Tests for the fix.
ilonatommy Feb 17, 2025
fc2e05e
Missing change to the backwards/forwards fix commit.
ilonatommy Feb 17, 2025
0a60851
Fix the test to pass in all cases.
ilonatommy Feb 17, 2025
d94b1af
Make the tests deterministic.
ilonatommy Feb 18, 2025
bd7858d
Restore enhanced nav asserts after the condition that full page is lo…
ilonatommy Feb 18, 2025
9a1d687
More tests revealing a problem with forwards/backwards action.
ilonatommy Feb 18, 2025
65cbf50
Let the browser handle history produced by forwards/backwards actions…
ilonatommy Feb 19, 2025
a8eb776
Missing change to the last commit.
ilonatommy Feb 20, 2025
47f06f5
Add programmatic navigation test cases.
ilonatommy Feb 20, 2025
3f032d2
Revert unnecessary changes.
ilonatommy Feb 20, 2025
eabd0f3
Update src/Components/test/E2ETest/ServerRenderingTests/EnhancedNavig…
ilonatommy Feb 20, 2025
634fa8e
Disable failing test cases
ilonatommy Feb 21, 2025
c973c00
Revert changes in old test assets and create new ones for scroll scen…
ilonatommy Feb 21, 2025
641ac54
Make sure buttons are visile before executing JS on them.
ilonatommy Feb 21, 2025
0d2b380
Typo
ilonatommy Feb 21, 2025
9c1e00e
Feedback.
ilonatommy Feb 21, 2025
c3c4368
Add retries to places that fail on CI.
ilonatommy Feb 24, 2025
fc26337
Merge branch 'main' into fix-51646
ilonatommy Feb 25, 2025
ba8dd72
Make flaky focus tests more deterministic and more verbose.
ilonatommy Feb 26, 2025
e755c48
Fix focus tests - change th order, first push to the history, then do…
ilonatommy Feb 27, 2025
1e5fb7c
Fix - browser handles the history if the enhanced load is done after …
ilonatommy Feb 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Components/Web.JS/src/Rendering/Renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export function resetScrollAfterNextBatch(): void {
shouldResetScrollAfterNextBatch = true;
}

function resetScrollIfNeeded() {
export function resetScrollIfNeeded() {
if (shouldResetScrollAfterNextBatch) {
shouldResetScrollAfterNextBatch = false;

Expand Down
17 changes: 16 additions & 1 deletion src/Components/Web.JS/src/Services/NavigationEnhancement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

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

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

function performProgrammaticEnhancedNavigation(absoluteInternalHref: string, replace: boolean) {
function performProgrammaticEnhancedNavigation(absoluteInternalHref: string, replace: boolean) : void {
const originalLocation = location.href;

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

if (!isForSamePath(absoluteInternalHref, originalLocation)) {
resetScrollAfterNextBatch();
}

performEnhancedPageLoad(absoluteInternalHref, /* interceptedLink */ false);
}

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

handleClickForNavigationInterception(event, absoluteInternalHref => {
const originalLocation = location.href;

const shouldScrollToHash = isSamePageWithHash(absoluteInternalHref);
history.pushState(null, /* ignored title */ '', absoluteInternalHref);

if (shouldScrollToHash) {
performScrollToElementOnTheSamePage(absoluteInternalHref);
} else {
let isSelfNavigation = isForSamePath(absoluteInternalHref, originalLocation);
performEnhancedPageLoad(absoluteInternalHref, /* interceptedLink */ true);
if (!isSelfNavigation) {
resetScrollAfterNextBatch();
resetScrollIfNeeded();
}
}
});
}
Expand All @@ -106,6 +120,7 @@ function onPopState(state: PopStateEvent) {
return;
}

// load the new page
performEnhancedPageLoad(location.href, /* interceptedLink */ false);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,70 @@
// The .NET Foundation licenses this file to you under the MIT license.

using OpenQA.Selenium;
using OpenQA.Selenium.Support.UI;
using System;

namespace Microsoft.AspNetCore.Components.E2ETest;

internal static class WebDriverExtensions
{
private static string GetFindPositionScript(string elementId) =>
$"return Math.round(document.getElementById('{elementId}').getBoundingClientRect().top + window.scrollY);";

public static void Navigate(this IWebDriver browser, Uri baseUri, string relativeUrl)
{
var absoluteUrl = new Uri(baseUri, relativeUrl);

browser.Navigate().GoToUrl("about:blank");
browser.Navigate().GoToUrl(absoluteUrl);
}

public static void WaitForElementToBeVisible(this IWebDriver browser, By by, int timeoutInSeconds = 5)
{
var wait = new DefaultWait<IWebDriver>(browser)
{
Timeout = TimeSpan.FromSeconds(timeoutInSeconds),
PollingInterval = TimeSpan.FromMilliseconds(100)
};
wait.IgnoreExceptionTypes(typeof(NoSuchElementException), typeof(StaleElementReferenceException));
wait.Until(driver =>
{
try
{
var element = driver.FindElement(by);
return element.Displayed;
}
catch (StaleElementReferenceException)
{
// Retry finding the element
return false;
}
});
}

public static long GetElementPositionWithRetry(this IWebDriver browser, string elementId, int retryCount = 3, int delayBetweenRetriesMs = 100)
{
var jsExecutor = (IJavaScriptExecutor)browser;
string script = GetFindPositionScript(elementId);
browser.WaitForElementToBeVisible(By.Id(elementId));
for (int i = 0; i < retryCount; i++)
{
try
{
var result = jsExecutor.ExecuteScript(script);
if (result != null)
{
return (long)result;
}
}
catch (OpenQA.Selenium.JavaScriptException)
{
// JavaScript execution failed, retry
}

Thread.Sleep(delayBetweenRetriesMs);
}

throw new Exception($"Failed to execute script after {retryCount} retries.");
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures;
using System.Threading.Tasks;
using Components.TestServer.RazorComponents;
using Microsoft.AspNetCore.Components.E2ETest;
using Microsoft.AspNetCore.Components.E2ETest.Infrastructure;
using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures;
using Microsoft.AspNetCore.E2ETesting;
using TestServer;
using Xunit.Abstractions;
using Components.TestServer.RazorComponents;
using OpenQA.Selenium;
using OpenQA.Selenium.BiDi.Communication;
using OpenQA.Selenium.Support.Extensions;
using TestServer;
using Xunit.Abstractions;
using Xunit.Sdk;

namespace Microsoft.AspNetCore.Components.E2ETests.ServerRenderingTests;

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

[Theory]
[InlineData(false, false, false)]
[InlineData(false, true, false)]
[InlineData(true, true, false)]
[InlineData(true, false, false)]
// [InlineData(false, false, true)] programmatic navigation doesn't work without enhanced navigation
[InlineData(false, true, true)]
[InlineData(true, true, true)]
// [InlineData(true, false, true)] programmatic navigation doesn't work without enhanced navigation
public void EnhancedNavigationScrollBehavesSameAsBrowserOnNavigation(bool enableStreaming, bool useEnhancedNavigation, bool programmaticNavigation)
{
// This test checks if the navigation to another path moves the scroll to the top of the page,
// or to the beginning of a fragment, regardless of the previous scroll position
string landingPageSuffix = enableStreaming ? "" : "-no-streaming";
string buttonKeyword = programmaticNavigation ? "-programmatic" : "";
Navigate($"{ServerPathBase}/nav/scroll-test{landingPageSuffix}");
EnhancedNavigationTestUtil.SuppressEnhancedNavigation(this, shouldSuppress: !useEnhancedNavigation, skipNavigation: true);

// "landing" page: scroll maximally down and go to "next" page - we should land at the top of that page
AssertWeAreOnLandingPage();

// staleness check is used to assert enhanced navigation is enabled/disabled, as requested
var elementForStalenessCheckOnNextPage = Browser.Exists(By.TagName("html"));

var button1Id = $"do{buttonKeyword}-navigation";
var button1Pos = Browser.GetElementPositionWithRetry(button1Id);
Browser.SetScrollY(button1Pos);
Browser.Exists(By.Id(button1Id)).Click();

// "next" page: check if we landed at 0, then navigate to "landing"
AssertWeAreOnNextPage();
WaitStreamingRendersFullPage(enableStreaming);
string fragmentId = "some-content";
Browser.WaitForElementToBeVisible(By.Id(fragmentId));
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnNextPage);
Assert.Equal(0, Browser.GetScrollY());
var elementForStalenessCheckOnLandingPage = Browser.Exists(By.TagName("html"));
var fragmentScrollPosition = Browser.GetElementPositionWithRetry(fragmentId);
Browser.Exists(By.Id(button1Id)).Click();

// "landing" page: navigate to a fragment on another page - we should land at the beginning of the fragment
AssertWeAreOnLandingPage();
WaitStreamingRendersFullPage(enableStreaming);
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnLandingPage);

var button2Id = $"do{buttonKeyword}-navigation-with-fragment";
Browser.Exists(By.Id(button2Id)).Click();
AssertWeAreOnNextPage();
WaitStreamingRendersFullPage(enableStreaming);
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnNextPage);
var expectedFragmentScrollPosition = fragmentScrollPosition - 1;
Assert.Equal(expectedFragmentScrollPosition, Browser.GetScrollY());
}

[Theory]
[InlineData(false, false, false)]
[InlineData(false, true, false)]
[InlineData(true, true, false)]
[InlineData(true, false, false)]
// [InlineData(false, false, true)] programmatic navigation doesn't work without enhanced navigation
[InlineData(false, true, true)]
[InlineData(true, true, true)]
// [InlineData(true, false, true)] programmatic navigation doesn't work without enhanced navigation
public void EnhancedNavigationScrollBehavesSameAsBrowserOnBackwardsForwardsAction(bool enableStreaming, bool useEnhancedNavigation, bool programmaticNavigation)
{
// This test checks if the scroll position is preserved after backwards/forwards action
string landingPageSuffix = enableStreaming ? "" : "-no-streaming";
string buttonKeyword = programmaticNavigation ? "-programmatic" : "";
Navigate($"{ServerPathBase}/nav/scroll-test{landingPageSuffix}");
EnhancedNavigationTestUtil.SuppressEnhancedNavigation(this, shouldSuppress: !useEnhancedNavigation, skipNavigation: true);

// "landing" page: scroll to pos1, navigate away
AssertWeAreOnLandingPage();
WaitStreamingRendersFullPage(enableStreaming);

// staleness check is used to assert enhanced navigation is enabled/disabled, as requested
var elementForStalenessCheckOnNextPage = Browser.Exists(By.TagName("html"));

var buttonId = $"do{buttonKeyword}-navigation";
Browser.WaitForElementToBeVisible(By.Id(buttonId));
var landingPagePos1 = Browser.GetElementPositionWithRetry(buttonId) - 100;
Browser.SetScrollY(landingPagePos1);
Browser.Exists(By.Id(buttonId)).Click();

// "next" page: scroll to pos1, navigate away
AssertWeAreOnNextPage();
WaitStreamingRendersFullPage(enableStreaming);
Browser.WaitForElementToBeVisible(By.Id(buttonId));
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnNextPage);
var elementForStalenessCheckOnLandingPage = Browser.Exists(By.TagName("html"));
var nextPagePos1 = Browser.GetElementPositionWithRetry(buttonId) - 100;
// make sure we are expecting different scroll positions on the 1st and the 2nd page
Assert.NotEqual(landingPagePos1, nextPagePos1);
Browser.SetScrollY(nextPagePos1);
Browser.Exists(By.Id(buttonId)).Click();

// "landing" page: scroll to pos2, go backwards
AssertWeAreOnLandingPage();
WaitStreamingRendersFullPage(enableStreaming);
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnLandingPage);
var landingPagePos2 = 500;
Browser.SetScrollY(landingPagePos2);
Browser.Navigate().Back();

// "next" page: check if we landed on pos1, move the scroll to pos2, go backwards
AssertWeAreOnNextPage();
WaitStreamingRendersFullPage(enableStreaming);
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnNextPage);
AssertScrollPositionCorrect(useEnhancedNavigation, nextPagePos1);

var nextPagePos2 = 600;
Browser.SetScrollY(nextPagePos2);
Browser.Navigate().Back();

// "landing" page: check if we landed on pos1, move the scroll to pos3, go forwards
AssertWeAreOnLandingPage();
WaitStreamingRendersFullPage(enableStreaming);
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnLandingPage);
AssertScrollPositionCorrect(useEnhancedNavigation, landingPagePos1);
var landingPagePos3 = 700;
Browser.SetScrollY(landingPagePos3);
Browser.Navigate().Forward();

// "next" page: check if we landed on pos1, go forwards
AssertWeAreOnNextPage();
WaitStreamingRendersFullPage(enableStreaming);
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnNextPage);
AssertScrollPositionCorrect(useEnhancedNavigation, nextPagePos2);
Browser.Navigate().Forward();

// "scroll" page: check if we landed on pos2
AssertWeAreOnLandingPage();
WaitStreamingRendersFullPage(enableStreaming);
AssertEnhancedNavigation(useEnhancedNavigation, elementForStalenessCheckOnLandingPage);
AssertScrollPositionCorrect(useEnhancedNavigation, landingPagePos2);
}

private void AssertScrollPositionCorrect(bool useEnhancedNavigation, long previousScrollPosition)
{
// from some reason, scroll position sometimes differs by 1 pixel between enhanced and browser's navigation
// browser's navigation is not precisely going backwards/forwards to the previous state
var currentScrollPosition = Browser.GetScrollY();
string messagePart = useEnhancedNavigation ? $"{previousScrollPosition}" : $"{previousScrollPosition} or {previousScrollPosition - 1}";
bool isPreciselyWhereItWasLeft = currentScrollPosition == previousScrollPosition;
bool isPixelLowerThanItWasLeft = currentScrollPosition == (previousScrollPosition - 1);
bool success = useEnhancedNavigation
? isPreciselyWhereItWasLeft
: (isPreciselyWhereItWasLeft || isPixelLowerThanItWasLeft);
Assert.True(success, $"The expected scroll position was {messagePart}, but it was found at {currentScrollPosition}.");
}

private void AssertEnhancedNavigation(bool useEnhancedNavigation, IWebElement elementForStalenessCheck, int retryCount = 3, int delayBetweenRetriesMs = 100)
{
bool enhancedNavigationDetected = false;
for (int i = 0; i < retryCount; i++)
{
try
{
enhancedNavigationDetected = !IsElementStale(elementForStalenessCheck);
Assert.Equal(useEnhancedNavigation, enhancedNavigationDetected);
return;
}
catch (XunitException)
{
// Maybe the check was done too early to change the DOM ref, retry
}

Thread.Sleep(delayBetweenRetriesMs);
}
string expectedNavigation = useEnhancedNavigation ? "enhanced navigation" : "browser navigation";
string isStale = enhancedNavigationDetected ? "is not stale" : "is stale";
var isNavigationSupressed = (string)((IJavaScriptExecutor)Browser).ExecuteScript("return sessionStorage.getItem('suppress-enhanced-navigation');");
throw new Exception($"Expected to use {expectedNavigation} because 'suppress-enhanced-navigation' is set to {isNavigationSupressed} but the element from previous path {isStale}");
}

private void AssertWeAreOnLandingPage()
{
string infoName = "test-info-1";
Browser.WaitForElementToBeVisible(By.Id(infoName), timeoutInSeconds: 20);
Browser.Equal("Scroll tests landing page", () => Browser.Exists(By.Id(infoName)).Text);
}

private void AssertWeAreOnNextPage()
{
string infoName = "test-info-2";
Browser.WaitForElementToBeVisible(By.Id(infoName), timeoutInSeconds: 20);
Browser.Equal("Scroll tests next page", () => Browser.Exists(By.Id(infoName)).Text);
}

private void WaitStreamingRendersFullPage(bool enableStreaming)
{
if (enableStreaming)
{
Browser.WaitForElementToBeVisible(By.Id("some-content"));
}
}

private void AssertEnhancedUpdateCountEquals(long count)
=> Browser.Equal(count, () => ((IJavaScriptExecutor)Browser).ExecuteScript("return window.enhancedPageUpdateCount;"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,7 @@ public static void SuppressEnhancedNavigation<TServerFixture>(ServerTestBase<TSe

public static long GetScrollY(this IWebDriver browser)
=> Convert.ToInt64(((IJavaScriptExecutor)browser).ExecuteScript("return window.scrollY"), CultureInfo.CurrentCulture);

public static long SetScrollY(this IWebDriver browser, long value)
=> Convert.ToInt64(((IJavaScriptExecutor)browser).ExecuteScript($"window.scrollTo(0, {value})"), CultureInfo.CurrentCulture);
}
Loading
Loading