diff --git a/app/assets/javascripts/lib/utils/resize_observer.js b/app/assets/javascripts/lib/utils/resize_observer.js
index ded1080ee813a5adc7e8bd0c1b4e9bfaff1346bc..995439a5fc3a485e7b3b473c74abc677c5e4e356 100644
--- a/app/assets/javascripts/lib/utils/resize_observer.js
+++ b/app/assets/javascripts/lib/utils/resize_observer.js
@@ -12,6 +12,7 @@ export function createResizeObserver() {
/**
* Watches for change in size of a container element (e.g. for lazy-loaded images)
* and scrolls the target note to the top of the content area.
+ * Stops watching if the target element is scrolled out of viewport
*
* @param {Object} options
* @param {string} options.targetId - id of element to scroll to
@@ -38,16 +39,21 @@ export function scrollToTargetOnResize({
// can't tell difference between user and el.scrollTo, so use a flag
let skipProgrammaticScrollEvent = false;
+ let intersectionObserver = null;
+ let targetElement = null;
+ let contentTopValue = contentTop();
+
const containerEl = document.querySelector(container);
const ro = createResizeObserver();
function handleScroll() {
if (skipProgrammaticScrollEvent) {
+ contentTopValue = contentTop();
skipProgrammaticScrollEvent = false;
return;
}
currentScrollPosition = scrollContainerIsDocument ? window.scrollY : scrollContainer.scrollTop;
- userScrollOffset = currentScrollPosition - targetTop;
+ userScrollOffset = currentScrollPosition - targetTop - contentTopValue;
}
function addScrollListener() {
@@ -67,16 +73,39 @@ export function scrollToTargetOnResize({
}
}
+ function setupIntersectionObserver() {
+ intersectionObserver = new IntersectionObserver(
+ (entries) => {
+ const [entry] = entries;
+
+ // if element gets scrolled off screen then remove listeners
+ if (!entry.isIntersecting) {
+ // eslint-disable-next-line no-use-before-define
+ cleanup();
+ }
+ },
+ {
+ root: scrollContainerIsDocument ? null : scrollContainer,
+ },
+ );
+
+ intersectionObserver.observe(targetElement);
+ }
+
function keepTargetAtTop() {
if (document.activeElement !== document.body) return;
const anchorEl = document.getElementById(targetId);
- if (!anchorEl) return;
+ if (!anchorEl) {
+ return;
+ }
scrollContainer = ScrollParent(document.getElementById(targetId)) || document.documentElement;
scrollContainerIsDocument = scrollContainer === document.documentElement;
- if (!scrollContainer) return;
+ if (!scrollContainer) {
+ return;
+ }
skipProgrammaticScrollEvent = true;
@@ -86,7 +115,7 @@ export function scrollToTargetOnResize({
// Add scrollPosition as getBoundingClientRect is relative to viewport
// Add the accumulated scroll offset to maintain relative position
// subtract contentTop so it goes below sticky headers, rather than top of viewport
- targetTop = anchorTop - contentTop() + currentScrollPosition + userScrollOffset;
+ targetTop = anchorTop - contentTopValue + currentScrollPosition + userScrollOffset;
scrollContainer.scrollTo({
top: targetTop,
@@ -97,18 +126,26 @@ export function scrollToTargetOnResize({
addScrollListener();
scrollListenerEnabled = true;
}
+
+ if (!intersectionObserver) {
+ targetElement = anchorEl;
+ setupIntersectionObserver();
+ }
+ }
+
+ function cleanup() {
+ ro.unobserve(containerEl);
+ containerEl.removeEventListener('ResizeUpdate', keepTargetAtTop);
+ removeScrollListener();
+
+ if (intersectionObserver) {
+ intersectionObserver.unobserve(targetElement);
+ intersectionObserver.disconnect();
+ }
}
containerEl.addEventListener('ResizeUpdate', keepTargetAtTop);
ro.observe(containerEl);
- return function cleanup() {
- // add a slight delay to this to allow for a final scroll to the
- // element once notes have finished
- setTimeout(() => {
- ro.unobserve(containerEl);
- containerEl.removeEventListener('ResizeUpdate', keepTargetAtTop);
- removeScrollListener();
- }, 100);
- };
+ return cleanup;
}
diff --git a/spec/frontend/lib/utils/resize_observer_spec.js b/spec/frontend/lib/utils/resize_observer_spec.js
index e2bbfeec5e60a421d09d61b1f3f2876c7279e74c..53e9866623959a03c92c0ca1f4b8e07db334ec3e 100644
--- a/spec/frontend/lib/utils/resize_observer_spec.js
+++ b/spec/frontend/lib/utils/resize_observer_spec.js
@@ -10,13 +10,14 @@ function mockStickyHeaderSize(val) {
describe('ResizeObserver Utility', () => {
let cleanup;
+ const mockHeaderSize = 90;
const triggerResize = () => {
const entry = document.querySelector('#content-body');
entry.dispatchEvent(new CustomEvent(`ResizeUpdate`, { detail: { entry } }));
};
beforeEach(() => {
- mockStickyHeaderSize(90);
+ mockStickyHeaderSize(mockHeaderSize);
jest.spyOn(document.documentElement, 'scrollTo');
@@ -34,83 +35,114 @@ describe('ResizeObserver Utility', () => {
resetHTMLFixture();
});
- describe('Observer behavior', () => {
- it('returns null for empty target', () => {
- cleanup = scrollToTargetOnResize({
- targetId: '',
- container: '#content-body',
- });
+ it('returns null for empty target', () => {
+ cleanup = scrollToTargetOnResize({
+ targetId: '',
+ container: '#content-body',
+ });
- expect(cleanup).toBe(null);
+ expect(cleanup).toBe(null);
+ });
+
+ it('does not scroll if target does not exist', () => {
+ scrollToTargetOnResize({
+ targetId: 'some_imaginary_id',
+ container: '#content-body',
});
- it('does not scroll if target does not exist', () => {
- scrollToTargetOnResize({
- targetId: 'some_imaginary_id',
+ triggerResize();
+
+ expect(document.documentElement.scrollTo).not.toHaveBeenCalled();
+ });
+
+ describe('with existing target', () => {
+ const topHeight = 110;
+ const scrollAmount = 160;
+
+ beforeEach(() => {
+ cleanup = scrollToTargetOnResize({
+ targetId: 'note_1234',
container: '#content-body',
});
+ });
+
+ it('returns cleanup function', () => {
+ cleanup();
triggerResize();
expect(document.documentElement.scrollTo).not.toHaveBeenCalled();
});
- describe('with existing target', () => {
- const cleanupTimeoutMs = 100;
- const topHeight = 110;
- const scrollAmount = 160;
+ it('scrolls body so anchor is just below sticky header (contentTop)', () => {
+ triggerResize();
- beforeEach(() => {
- cleanup = scrollToTargetOnResize({
- targetId: 'note_1234',
- container: '#content-body',
- });
+ expect(document.documentElement.scrollTo).toHaveBeenCalledWith({
+ behavior: 'instant',
+ top: topHeight,
});
+ });
- it('returns cleanup function', () => {
- cleanup();
+ it('maintains scroll position relative to anchor after user scroll', () => {
+ // Initial scroll to anchor
+ triggerResize();
- jest.advanceTimersByTime(cleanupTimeoutMs);
+ // Simulate user scrolling down
+ window.scrollY = scrollAmount;
+ window.dispatchEvent(new Event('scroll'));
- triggerResize();
+ // Trigger resize again
+ triggerResize();
- expect(document.documentElement.scrollTo).not.toHaveBeenCalled();
+ // Should maintain the 50px offset from original position
+ expect(document.documentElement.scrollTo).toHaveBeenCalledWith({
+ top: topHeight + scrollAmount,
+ behavior: 'instant',
});
+ });
- it('scrolls body so anchor is just below sticky header (contentTop)', () => {
- triggerResize();
+ it('does not scroll if another element is focused', () => {
+ const anchorEl = document.getElementById('reply-field');
+ anchorEl.focus();
- expect(document.documentElement.scrollTo).toHaveBeenCalledWith({
- behavior: 'instant',
- top: topHeight,
- });
- });
+ triggerResize();
- it('maintains scroll position relative to anchor after user scroll', () => {
- // Initial scroll to anchor
- triggerResize();
+ expect(document.documentElement.scrollTo).not.toHaveBeenCalled();
+ });
- // Simulate user scrolling down
- window.scrollY = scrollAmount;
- window.dispatchEvent(new Event('scroll'));
+ describe('intersection observer', () => {
+ let intersectionCallback;
+ let mockIntersectionObserver;
- // Trigger resize again
+ beforeEach(() => {
+ mockIntersectionObserver = jest.fn((callback) => {
+ intersectionCallback = callback;
+ return {
+ observe: jest.fn(),
+ unobserve: jest.fn(),
+ disconnect: jest.fn(),
+ };
+ });
+
+ global.IntersectionObserver = mockIntersectionObserver;
+ });
+
+ it('sets up intersection observer after first scroll', () => {
triggerResize();
- // Should maintain the 50px offset from original position
- expect(document.documentElement.scrollTo).toHaveBeenCalledWith({
- top: topHeight + scrollAmount,
- behavior: 'instant',
+ expect(mockIntersectionObserver).toHaveBeenCalled();
+ expect(mockIntersectionObserver.mock.calls[0][1]).toEqual({
+ root: null,
});
});
- it('does not scroll if another element is focused', () => {
- const anchorEl = document.getElementById('reply-field');
- anchorEl.focus();
-
+ it('cleans up when target is no longer visible', () => {
triggerResize();
- expect(document.documentElement.scrollTo).not.toHaveBeenCalled();
+ intersectionCallback([{ isIntersecting: false }]);
+
+ triggerResize();
+ expect(document.documentElement.scrollTo).toHaveBeenCalledTimes(1);
});
});
});