Scroll in enhanced navigation behaves same as in client side routing by ilonatommy · Pull Request #60296 · dotnet/aspnetcore

ilonatommy · 2025-02-10T14:02:01Z

Description

Navigation scrolls to the top -> see resetScrollAfterNextBatch() in enhanced nav
But it does not scroll if it's self-navigation (or self-navigation with query), to make enhanced nav behave the same as introduced in NavigateTo does not scroll to the top in internal navigation to same url #60190 for client side navigation.
Streaming rendering loads parts of the page with a delay, so the tests required a tool to make sure the full page is loaded: WaitForElementToBeVisible extension was added. Without it, the tests with enableStreaming=true would not be deterministic.
Enhanced backwards/forwards: the history produced by forwards/backwards actions are handled properly by the browser navigation, even when enhanced navigation is switched on.

Fixes #51646

…d scroll position.

…aded.

… and enhanced nav handle the history produced by onDocumentClick.

Copilot

Copilot reviewed 5 out of 9 changed files in this pull request and generated 1 comment.

Files not reviewed (4)

src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/EnhancedNav/PageForScrollPositionTests.razor: Evaluated as low risk
src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/EnhancedNav/PageForScrollPositionTestsNoStreaming.razor: Evaluated as low risk
src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/EnhancedNav/PageForScrollingToHash.razor: Evaluated as low risk
src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/EnhancedNav/PageForScrollingToHashNoStreaming.razor: Evaluated as low risk

Comments suppressed due to low confidence (1)

src/Components/Web.JS/src/Services/NavigationEnhancement.ts:98

The function saveScrollPosition should handle cases where the state is null or undefined. Add a check to ensure the state is correctly handled.

const currentState = history.state || {};

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

…ationTest.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

ilonatommy · 2025-02-20T15:25:44Z

Running the tests locally on repeat revealed an issue with browser's navigation. When we use programmatic navigation, it does not always load the new page (the history state changes but the DOM stays the same, we can see it as failure to render the element of id="test-info-hash", that should replace id="test-info-scroll"). It happens on the 1st run of the tests, with and without streaming and increasing the timeout does not impact it. On the next runs, the test case switches automatically to enhanced nav, even though "sessionStorage.setItem('suppress-enhanced-navigation', 'true')" is set at the beginning of the test.

List of affected test cases:

- Microsoft.AspNetCore.Components.E2ETests.ServerRenderingTests.EnhancedNavigationTest.EnhancedNavigationScrollBehavesSameAsBrowserOnBackwardsForwardsAction(enableStreaming: True, useEnhancedNavigation: False, programmaticNavigation: True) 
- Microsoft.AspNetCore.Components.E2ETests.ServerRenderingTests.EnhancedNavigationTest.EnhancedNavigationScrollBehavesSameAsBrowserOnBackwardsForwardsAction(enableStreaming: False, useEnhancedNavigation: False, programmaticNavigation: True) 
- Microsoft.AspNetCore.Components.E2ETests.ServerRenderingTests.EnhancedNavigationTest.EnhancedNavigationScrollBehavesSameAsBrowserOnNavigation(enableStreaming: True, useEnhancedNavigation: False, programmaticNavigation: True) 
- Microsoft.AspNetCore.Components.E2ETests.ServerRenderingTests.EnhancedNavigationTest.EnhancedNavigationScrollBehavesSameAsBrowserOnNavigation(enableStreaming: False, useEnhancedNavigation: False, programmaticNavigation: True)

error:

Message: 
OpenQA.Selenium.WebDriverTimeoutException : Timed out after 20 seconds
---- OpenQA.Selenium.NoSuchElementException : no such element: Unable to locate element: {"method":"css selector","selector":"#test\-info\-hash"}
  (Session info: chrome=133.0.6943.99); For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception

    Stack Trace: 
DefaultWait`1.ThrowTimeoutException(String exceptionMessage, Exception lastException)
DefaultWait`1.Until[TResult](Func`2 condition, CancellationToken token)
WebDriverExtensions.WaitForElementToBeVisible(IWebDriver browser, By by, Int32 timeoutInSeconds) line 28
EnhancedNavigationTest.AssertWeAreOnHashPage() line 834
EnhancedNavigationTest.EnhancedNavigationScrollBehavesSameAsBrowserOnBackwardsForwardsAction(Boolean enableStreaming, Boolean useEnhancedNavigation, Boolean programmaticNavigation) line 753
InvokeStub_EnhancedNavigationTest.EnhancedNavigationScrollBehavesSameAsBrowserOnBackwardsForwardsAction(Object, Span`1)
MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)

ilonatommy · 2025-02-21T09:25:11Z

The test cases that fall into this path:

aspnetcore/src/Components/Web.JS/src/Services/NavigationManager.ts

Line 122 in a2e3677

    
           performInternalNavigation(absoluteUri, false, options.replaceHistoryEntry, options.historyEntryState, skipLocationChangingCallback);

are failing. This is not an issue connected with this PR and it happens on main as well.

…arios.

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

javiercn

Looks great!

Good job on the testing aspects of this feature, I know this is not a simple area and the coverage looks thorough!

… enhanced load.

…the history update.

Fix.

d10ebfa

ilonatommy added area-blazor Includes: Blazor, Razor Components feature-blazor-enhanced-navigation labels Feb 10, 2025

ilonatommy self-assigned this Feb 10, 2025

ilonatommy added 7 commits February 10, 2025 16:36

Fix: move the check before location.href change.

06655d5

Fix: backwards/forwsrds button, preserve scroll position.

8da794c

Draft of tests.

97420c2

Fix: new page lands on the top + backwards/forwards lands on the save…

cfe8046

…d scroll position.

Tests for the fix.

d56bc30

Missing change to the backwards/forwards fix commit.

fc2e05e

Fix the test to pass in all cases.

0a60851

build-analysis bot mentioned this pull request Feb 18, 2025

error : HttpRequestException: The SSL connection could not be established, see inner exception. dotnet/dnceng#5015

Open

3 tasks

ilonatommy added 5 commits February 18, 2025 12:08

Make the tests deterministic.

d94b1af

Restore enhanced nav asserts after the condition that full page is lo…

bd7858d

…aded.

More tests revealing a problem with forwards/backwards action.

9a1d687

Let the browser handle history produced by forwards/backwards actions…

65cbf50

… and enhanced nav handle the history produced by onDocumentClick.

Missing change to the last commit.

a8eb776

ilonatommy marked this pull request as ready for review February 20, 2025 09:02

Copilot AI review requested due to automatic review settings February 20, 2025 09:02

ilonatommy requested a review from a team as a code owner February 20, 2025 09:02

ilonatommy requested a review from javiercn February 20, 2025 09:03

Copilot AI reviewed Feb 20, 2025

View reviewed changes

src/Components/test/E2ETest/ServerRenderingTests/EnhancedNavigationTest.cs Outdated Show resolved Hide resolved

ilonatommy marked this pull request as draft February 20, 2025 09:06

ilonatommy and others added 3 commits February 20, 2025 15:51

Add programmatic navigation test cases.

47f06f5

Revert unnecessary changes.

3f032d2

Update src/Components/test/E2ETest/ServerRenderingTests/EnhancedNavig…

eabd0f3

…ationTest.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

ilonatommy marked this pull request as ready for review February 20, 2025 15:28

Disable failing test cases

634fa8e

Revert changes in old test assets and create new ones for scroll scen…

c973c00

…arios.

javiercn reviewed Feb 21, 2025

View reviewed changes

src/Components/Web.JS/src/Services/NavigationEnhancement.ts Outdated Show resolved Hide resolved

javiercn approved these changes Feb 21, 2025

View reviewed changes

ilonatommy added 3 commits February 21, 2025 17:34

Make sure buttons are visile before executing JS on them.

641ac54

Typo

0d2b380

Feedback.

9c1e00e

build-analysis bot mentioned this pull request Feb 21, 2025

The active test run was aborted. Reason: Test host process crashed dotnet/dnceng#451

Closed

3 tasks

This was referenced Feb 24, 2025

Blazor .NET 10 Tracking Issue dotnet/AspNetCore.Docs#34437

Closed

NavigateTo no longer scrolls to the top for same page navigations dotnet/AspNetCore.Docs#34789

Closed

Add retries to places that fail on CI.

c3c4368

ilonatommy enabled auto-merge (squash) February 24, 2025 13:58

ilonatommy added 4 commits February 25, 2025 15:31

Merge branch 'main' into fix-51646

fc26337

Make flaky focus tests more deterministic and more verbose.

ba8dd72

Fix focus tests - change th order, first push to the history, then do…

e755c48

… enhanced load.

Fix - browser handles the history if the enhanced load is done after …

1e5fb7c

…the history update.

ilonatommy merged commit 719c33f into dotnet:main Feb 27, 2025
27 checks passed

dotnet-policy-service bot added this to the 10.0-preview3 milestone Feb 27, 2025

ilonatommy mentioned this pull request Mar 13, 2025

Quarantine RadioButtonGetsResetAfterSubmittingEnhancedForm #60873

Closed

ilonatommy mentioned this pull request Aug 4, 2025

SSR: internal a href links do not scroll to the top of the new page when clicked #53459

Closed

1 task

GiampaoloGabba mentioned this pull request Oct 13, 2025

Enhanced navigation in .NET10: scroll happens before content renders, causing visual flash #64015

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

Scroll in enhanced navigation behaves same as in client side routing#60296
ilonatommy merged 26 commits intodotnet:mainfrom
ilonatommy:fix-51646

ilonatommy commented Feb 10, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

ilonatommy commented Feb 20, 2025

Uh oh!

ilonatommy commented Feb 21, 2025

Uh oh!

Uh oh!

javiercn left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ilonatommy commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ilonatommy commented Feb 20, 2025

Uh oh!

ilonatommy commented Feb 21, 2025

Uh oh!

Uh oh!

javiercn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ilonatommy commented Feb 10, 2025 •

edited

Loading