Skip to content

Commit 6e52c9f

Browse files
committed
Feedback
1 parent 5d3a076 commit 6e52c9f

File tree

5 files changed

+41
-32
lines changed

5 files changed

+41
-32
lines changed

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

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

44
import { synchronizeDomContent } from '../Rendering/DomMerging/DomSync';
5-
import { attachProgrammaticEnhancedNavigationHandler, handleClickForNavigationInterception, hasInteractiveRouter, isForSamePath, isSamePageWithHash, notifyEnhancedNavigationListeners, performScrollToElementOnTheSamePage, isHashOnlyChange } from './NavigationUtils';
5+
import { attachProgrammaticEnhancedNavigationHandler, handleClickForNavigationInterception, hasInteractiveRouter, isForSamePath, notifyEnhancedNavigationListeners, performScrollToElementOnTheSamePage, isSameUrlWithDifferentHash } from './NavigationUtils';
66
import { resetScrollAfterNextBatch, resetScrollIfNeeded } from '../Rendering/Renderer';
77

88
/*
@@ -99,7 +99,7 @@ function onDocumentClick(event: MouseEvent) {
9999
handleClickForNavigationInterception(event, absoluteInternalHref => {
100100
const originalLocation = location.href;
101101

102-
const shouldScrollToHash = isSamePageWithHash(absoluteInternalHref);
102+
const shouldScrollToHash = isSameUrlWithDifferentHash(originalLocation, absoluteInternalHref);
103103
history.pushState(null, /* ignored title */ '', absoluteInternalHref);
104104

105105
if (shouldScrollToHash) {
@@ -120,7 +120,7 @@ function onPopState(state: PopStateEvent) {
120120
return;
121121
}
122122

123-
if (isHashOnlyChange(currentContentUrl, location.href)) {
123+
if (isSameUrlWithDifferentHash(currentContentUrl, location.href)) {
124124
return;
125125
}
126126

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import '@microsoft/dotnet-js-interop';
55
import { resetScrollAfterNextBatch } from '../Rendering/Renderer';
66
import { EventDelegator } from '../Rendering/Events/EventDelegator';
7-
import { attachEnhancedNavigationListener, getInteractiveRouterRendererId, handleClickForNavigationInterception, hasInteractiveRouter, hasProgrammaticEnhancedNavigationHandler, isForSamePath, isSamePageWithHash, isWithinBaseUriSpace, performProgrammaticEnhancedNavigation, performScrollToElementOnTheSamePage, scrollToElement, setHasInteractiveRouter, toAbsoluteUri } from './NavigationUtils';
7+
import { attachEnhancedNavigationListener, getInteractiveRouterRendererId, handleClickForNavigationInterception, hasInteractiveRouter, hasProgrammaticEnhancedNavigationHandler, isForSamePath, isSameUrlWithDifferentHash, isWithinBaseUriSpace, performProgrammaticEnhancedNavigation, performScrollToElementOnTheSamePage, scrollToElement, setHasInteractiveRouter, toAbsoluteUri } from './NavigationUtils';
88
import { WebRendererId } from '../Rendering/WebRendererId';
99
import { isRendererAttached } from '../Rendering/WebRendererInteropMethods';
1010

@@ -150,7 +150,7 @@ function performExternalNavigation(uri: string, replace: boolean) {
150150
async function performInternalNavigation(absoluteInternalHref: string, interceptedLink: boolean, replace: boolean, state: string | undefined = undefined, skipLocationChangingCallback = false) {
151151
ignorePendingNavigation();
152152

153-
if (isSamePageWithHash(absoluteInternalHref)) {
153+
if (isSameUrlWithDifferentHash(location.href, absoluteInternalHref)) {
154154
saveToBrowserHistory(absoluteInternalHref, replace, state);
155155
performScrollToElementOnTheSamePage(absoluteInternalHref);
156156
return;

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,12 @@ export function isWithinBaseUriSpace(href: string) {
4747
&& (nextChar === '' || nextChar === '/' || nextChar === '?' || nextChar === '#');
4848
}
4949

50-
export function isSamePageWithHash(absoluteHref: string): boolean {
51-
const url = new URL(absoluteHref);
52-
return url.hash !== '' && location.origin === url.origin && location.pathname === url.pathname && location.search === url.search;
53-
}
54-
55-
export function isHashOnlyChange(oldUrl: string, newUrl: string): boolean {
50+
export function isSameUrlWithDifferentHash(oldUrl: string, newUrl: string): boolean {
5651
try {
5752
const a = new URL(oldUrl);
5853
const b = new URL(newUrl);
5954
return a.origin === b.origin && a.pathname === b.pathname
60-
&& a.search === b.search && a.hash !== b.hash;
55+
&& a.search === b.search && b.hash !== '';
6156
} catch {
6257
return false;
6358
}

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

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -197,31 +197,37 @@ public void CanScrollToHashWithoutPerformingFullNavigation()
197197
}
198198

199199
[Fact]
200-
public void NonEnhancedNavCanScrollToHashWithoutFetchingPage()
200+
public void NonEnhancedNavCanScrollToHashWithoutFetchingPageAnchor()
201201
{
202202
Navigate($"{ServerPathBase}/nav/scroll-to-hash");
203-
Browser.Equal("Scroll to hash", () => Browser.Exists(By.TagName("h1")).Text);
204-
205-
var javascript = (IJavaScriptExecutor)Browser;
206-
javascript.ExecuteScript(@"
207-
window.testFetchCalls = [];
208-
const originalFetch = window.fetch;
209-
window.fetch = function(...args) {
210-
window.testFetchCalls.push(args[0]);
211-
return originalFetch.apply(this, args);
212-
};");
203+
var originalTextElem = Browser.Exists(By.CssSelector("#anchor #text"));
204+
Browser.Equal("Text", () => originalTextElem.Text);
213205

214-
Browser.Exists(By.Id("scroll-anchor-enhance-nav-false")).Click();
206+
Browser.Exists(By.CssSelector("#anchor #scroll-anchor")).Click();
215207
Browser.True(() => Browser.GetScrollY() > 500);
216208
Browser.True(() => Browser
217-
.Exists(By.Id("uri-on-page-load-enhance-nav-false"))
209+
.Exists(By.CssSelector("#anchor #uri-on-page-load"))
218210
.GetDomAttribute("data-value")
219211
.EndsWith("scroll-to-hash", StringComparison.Ordinal));
220212

221-
var fetchCalls = javascript.ExecuteScript("return window.testFetchCalls;") as IEnumerable<object>;
222-
var relevantCalls = fetchCalls?.Where(call => call.ToString().Contains("scroll-to-hash")) ?? Enumerable.Empty<object>();
213+
Browser.Equal("Text", () => originalTextElem.Text);
214+
}
215+
216+
[Fact]
217+
public void NonEnhancedNavCanScrollToHashWithoutFetchingPageNavLink()
218+
{
219+
Navigate($"{ServerPathBase}/nav/scroll-to-hash");
220+
var originalTextElem = Browser.Exists(By.CssSelector("#navlink #text"));
221+
Browser.Equal("Text", () => originalTextElem.Text);
222+
223+
Browser.Exists(By.CssSelector("#navlink #scroll-anchor")).Click();
224+
Browser.True(() => Browser.GetScrollY() > 500);
225+
Browser.True(() => Browser
226+
.Exists(By.CssSelector("#navlink #uri-on-page-load"))
227+
.GetDomAttribute("data-value")
228+
.EndsWith("scroll-to-hash", StringComparison.Ordinal));
223229

224-
Assert.Empty(relevantCalls);
230+
Browser.Equal("Text", () => originalTextElem.Text);
225231
}
226232

227233
[Theory]

src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/EnhancedNav/PageForScrollingToHash.razor

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
@page "/nav/scroll-to-hash"
22
@attribute [StreamRendering]
33
@inject NavigationManager NavigationManager
4+
@using Microsoft.AspNetCore.Components.Forms
45

56
<PageTitle>Page for scrolling to hash</PageTitle>
67

@@ -13,10 +14,17 @@
1314
<div id="uri-on-page-load" style="display: none" data-value="@uriOnPageLoad"></div>
1415
</p>
1516

16-
<p data-enhance-nav="false">
17-
<a id="scroll-anchor-enhance-nav-false" href="nav/scroll-to-hash#some-content">Scroll via anchor</a>
18-
<div id="uri-on-page-load-enhance-nav-false" style="display: none" data-value="@uriOnPageLoad"></div>
19-
</p>
17+
<div data-enhance-nav="false" id="anchor">
18+
<a id="scroll-anchor" href="nav/scroll-to-hash#some-content">Scroll via anchor</a>
19+
<div id="uri-on-page-load" style="display: none" data-value="@uriOnPageLoad"></div>
20+
<p id="text">Text</p>
21+
</div>
22+
23+
<div data-enhance-nav="false" id="navlink">
24+
<NavLink id="scroll-anchor" href="nav/scroll-to-hash#some-content">Scroll via NavLink</NavLink>
25+
<div id="uri-on-page-load" style="display: none" data-value="@uriOnPageLoad"></div>
26+
<p id="text">Text</p>
27+
</div>
2028

2129
<div style="height: 2000px; border: 2px dashed red;">spacer</div>
2230

0 commit comments

Comments
 (0)