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

fix: when query parameters are changed on a page - ilc still tries to scroll to top. #543

Merged
merged 2 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
export class UrlHashController {
export class ScrollController {
// @todo write e2e tests for hash position restoring
#hashStoreNode = document.body;
#hashStoreAttribute = 'ilcTempStoredHash';
#lastVisitedUrl = this.location.pathname;
#shouldScrollToTop = true;

get location() {
return window.location;
}
blackrabbit99 marked this conversation as resolved.
Show resolved Hide resolved

store() {
if (window.location.hash) {
this.#shouldScrollToTop = this.#lastVisitedUrl.toLowerCase() !== this.location.pathname.toLowerCase();
this.#lastVisitedUrl = this.location.pathname;
if (this.location.hash) {
const node = this.#hashStoreNode;
const hashValue = this.#getHashValue();
node.setAttribute(this.#hashStoreAttribute, hashValue);
Expand All @@ -15,26 +23,28 @@ export class UrlHashController {
const node = this.#hashStoreNode;
// @todo: looks like it never used so storing is useless
node.removeAttribute(this.#hashStoreAttribute);
this.#scrollToHash();
this.#restoreScrollOnNavigation();
}
blackrabbit99 marked this conversation as resolved.
Show resolved Hide resolved

#scrollToHash() {
#restoreScrollOnNavigation() {
let scrollToElement;

if (window.location.hash) {
scrollToElement = document.querySelector(window.location.hash);
if (this.location.hash) {
scrollToElement = document.getElementById(this.#getHashValue());
}

if (scrollToElement) {
scrollToElement.scrollIntoView();
} else {
return;
}

if (this.#shouldScrollToTop) {
window.scroll(0, 0);
}
}

#getHashValue() {
try {
return window.location.hash.slice(1);
return decodeURIComponent(this.location.hash.slice(1));
} catch (error) {
// @todo handle an error the hash should not be read if it not exists
return false;
Expand Down
112 changes: 112 additions & 0 deletions ilc/client/TransitionManager/ScrollController/ScrollController.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import { expect } from 'chai';
import { ScrollController } from './ScrollController';
import sinon from 'sinon';

describe('ScrollController', () => {
let scrollController;
let testAnchor;

beforeEach(() => {
// Create and append a specific element to the body for the test
testAnchor = document.createElement('div');
testAnchor.id = 'testAnchor';
document.body.appendChild(testAnchor);

scrollController = new ScrollController();
});

afterEach(() => {
// Clean up the DOM by removing the test element after each test
document.body.removeChild(testAnchor);
document.body.removeAttribute('ilcTempStoredHash');
window.location.hash = ''; // Reset the hash
});

describe('store', () => {
it('should store the current window hash when called', () => {
window.location.hash = '#testAnchor';
scrollController.store();
expect(document.body.getAttribute('ilcTempStoredHash')).to.equal('testAnchor');
});

it('should not store the hash if it is not present', () => {
scrollController.store();
expect(document.body.hasAttribute('ilcTempStoredHash')).to.be.false;
});
});

describe('restore', () => {
let scrollSpy;
let scrollIntoViewSpy;

afterEach(() => {
if (scrollSpy) {
scrollSpy.restore();
scrollSpy = null;
}

if (scrollIntoViewSpy) {
scrollIntoViewSpy.restore();
scrollIntoViewSpy = null;
}
});

it('should remove the stored hash attribute from the document body', () => {
document.body.setAttribute('ilcTempStoredHash', 'testAnchor');
scrollController.restore();
expect(document.body.hasAttribute('ilcTempStoredHash')).to.be.false;
});

it('should scroll to the element associated with the current hash', () => {
scrollSpy = sinon.spy(window, 'scroll');
scrollIntoViewSpy = sinon.spy(testAnchor, 'scrollIntoView');

window.location.hash = '#testAnchor';
scrollController.restore();

sinon.assert.calledOnce(testAnchor.scrollIntoView);
sinon.assert.notCalled(window.scroll);
});

it('should scroll to top if no element is associated with the current hash', () => {
scrollSpy = sinon.spy(window, 'scroll');

window.location.hash = '#nonExistentAnchor';
scrollController.restore();

sinon.assert.calledWith(window.scroll, 0, 0);
});

it('should not scroll to top if store has recorded navigation between the same path', () => {
scrollSpy = sinon.spy(window, 'scroll');
scrollController.store();
scrollController.store();

scrollController.restore();

sinon.assert.notCalled(window.scroll);
});

it('should handle hashes that need to be decoded', () => {
const encodedHash = '#test%20anchor'; // Encoded "#test anchor"
document.getElementById('testAnchor').id = 'test anchor';
window.location.hash = encodedHash;
scrollIntoViewSpy = sinon.spy(testAnchor, 'scrollIntoView');

try {
scrollController.restore();
sinon.assert.calledOnce(scrollIntoViewSpy);
} finally {
document.getElementById('test anchor').id = 'testAnchor';
}
});

it('should safely handle hashes with characters that need escaping', () => {
// Simulate a hash that needs escaping
const invalidHash = '#test:id';
window.location.hash = invalidHash;

expect(() => scrollController.restore()).to.not.throw();
});
});
});
8 changes: 4 additions & 4 deletions ilc/client/TransitionManager/TransitionManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import ilcEvents from '../constants/ilcEvents';
import TransitionBlockerList from './TransitionBlockerList';
import { CssTrackedApp } from '../CssTrackedApp';
import { GlobalSpinner } from './GlobalSpinner/GlobalSpinner';
import { UrlHashController } from './UrlHashController/UrlHashController';
import { ScrollController } from './ScrollController/ScrollController';
import { SlotRenderObserver } from './SlotRenderObserver/SlotRenderObserver';

export const slotWillBe = {
Expand Down Expand Up @@ -34,7 +34,7 @@ export class TransitionManager {

/** @type TransitionBlockerList */
#transitionBlockers = new TransitionBlockerList();
#urlHashController = new UrlHashController();
#scrollController = new ScrollController();

constructor(logger, spinnerConfig) {
this.#logger = logger;
Expand Down Expand Up @@ -115,7 +115,7 @@ export class TransitionManager {

const contentListenerBlocker = new NamedTransactionBlocker(slotName, (resolve) => {
this.#runGlobalSpinner();
this.#urlHashController.store();
this.#scrollController.store();

const targetNode = getSlotElement(slotName);
targetNode.style.display = 'none'; // we will show all new slots, only when all will be settled
Expand Down Expand Up @@ -166,7 +166,7 @@ export class TransitionManager {
this.#hiddenSlots.length = 0;

this.#removeGlobalSpinner();
this.#urlHashController.restore();
this.#scrollController.restore();

window.dispatchEvent(new CustomEvent(ilcEvents.PAGE_READY));
};
Expand Down
5 changes: 1 addition & 4 deletions registry/server/templates/services/resources/Resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ export abstract class Resource {
...Attributes.crossorigin,
};

constructor(
public uri: string,
params?: Params,
) {
constructor(public uri: string, params?: Params) {
this.params = params || {};
}

Expand Down
11 changes: 4 additions & 7 deletions registry/server/templates/services/templatesRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,10 @@ export async function readTemplateWithAllVersions(templateName: string) {
.select()
.from<LocalizedTemplate>(tables.templatesLocalized)
.where('templateName', templateName);
template.localizedVersions = localizedTemplates.reduce(
(acc, item) => {
acc[item.locale] = { content: item.content };
return acc;
},
{} as Record<string, object>,
);
template.localizedVersions = localizedTemplates.reduce((acc, item) => {
acc[item.locale] = { content: item.content };
return acc;
}, {} as Record<string, object>);

return template;
}
Expand Down
Loading