From 2f15a2df25a341944e6c79e0e007f2082527f2f6 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Mon, 9 Mar 2026 17:42:00 +0100 Subject: [PATCH] Fix scroll-to-hint checking visibility of zero-height marker instead of parent widget (#25257) Pass the parent element to `isElementVisibleInContainer` so the scroll hint correctly detects when a newly created widget is not fully visible in the viewport. Previously the empty marker div was considered visible even when the actual widget content was off-screen. Fixes #25237 Co-authored-by: Claude Opus 4.6 --- changelog/unreleased/issue-25237.toml | 5 ++ .../components/common/ScrollToHint.test.tsx | 77 +++++++++++++++++++ .../views/components/common/ScrollToHint.tsx | 8 +- 3 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 changelog/unreleased/issue-25237.toml create mode 100644 graylog2-web-interface/src/views/components/common/ScrollToHint.test.tsx diff --git a/changelog/unreleased/issue-25237.toml b/changelog/unreleased/issue-25237.toml new file mode 100644 index 0000000000..d50dd9a49c --- /dev/null +++ b/changelog/unreleased/issue-25237.toml @@ -0,0 +1,5 @@ +type = "fixed" +message = "Fix scrolling to newly created widgets by checking visibility of the correct elements." + +issues = ["25237"] +pulls = ["25257"] diff --git a/graylog2-web-interface/src/views/components/common/ScrollToHint.test.tsx b/graylog2-web-interface/src/views/components/common/ScrollToHint.test.tsx new file mode 100644 index 0000000000..b091f9ef10 --- /dev/null +++ b/graylog2-web-interface/src/views/components/common/ScrollToHint.test.tsx @@ -0,0 +1,77 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +import { isElementVisibleInContainer } from './ScrollToHint'; + +describe('isElementVisibleInContainer', () => { + const mockElement = (rect: Partial): HTMLElement => + ({ + getBoundingClientRect: () => ({ + top: 0, + bottom: 0, + left: 0, + right: 0, + width: 0, + height: 0, + x: 0, + y: 0, + toJSON: () => {}, + ...rect, + }), + }) as unknown as HTMLElement; + + it('returns true when the target is fully visible inside the container', () => { + const container = mockElement({ top: 0, bottom: 500 }); + const target = mockElement({ top: 100, bottom: 200 }); + + expect(isElementVisibleInContainer(target, container)).toBe(true); + }); + + it('returns false when the target is below the container', () => { + const container = mockElement({ top: 0, bottom: 500 }); + const target = mockElement({ top: 600, bottom: 700 }); + + expect(isElementVisibleInContainer(target, container)).toBe(false); + }); + + it('returns false when the target is above the container', () => { + const container = mockElement({ top: 100, bottom: 500 }); + const target = mockElement({ top: 0, bottom: 50 }); + + expect(isElementVisibleInContainer(target, container)).toBe(false); + }); + + it('returns false when the target is partially visible at the bottom', () => { + const container = mockElement({ top: 0, bottom: 500 }); + const target = mockElement({ top: 450, bottom: 600 }); + + expect(isElementVisibleInContainer(target, container)).toBe(false); + }); + + it('returns false when the target is partially visible at the top', () => { + const container = mockElement({ top: 100, bottom: 500 }); + const target = mockElement({ top: 50, bottom: 200 }); + + expect(isElementVisibleInContainer(target, container)).toBe(false); + }); + + it('returns true when the target exactly matches the container bounds', () => { + const container = mockElement({ top: 0, bottom: 500 }); + const target = mockElement({ top: 0, bottom: 500 }); + + expect(isElementVisibleInContainer(target, container)).toBe(true); + }); +}); diff --git a/graylog2-web-interface/src/views/components/common/ScrollToHint.tsx b/graylog2-web-interface/src/views/components/common/ScrollToHint.tsx index 4541a32aa1..5d94842069 100644 --- a/graylog2-web-interface/src/views/components/common/ScrollToHint.tsx +++ b/graylog2-web-interface/src/views/components/common/ScrollToHint.tsx @@ -40,7 +40,7 @@ const ScrollHint = styled.button( `, ); -const isElementVisibleInContainer = (target: HTMLElement, scrollContainer: HTMLElement) => { +export const isElementVisibleInContainer = (target: HTMLElement, scrollContainer: HTMLElement) => { const containerRect = scrollContainer.getBoundingClientRect(); const targetRect = target.getBoundingClientRect(); @@ -82,7 +82,10 @@ const ScrollToHint = ({ ifTrue && scrollTargetRef.current && scrollContainer.current && - !isElementVisibleInContainer(scrollTargetRef.current, scrollContainer.current) + !isElementVisibleInContainer( + scrollTargetRef.current.parentElement ?? scrollTargetRef.current, + scrollContainer.current, + ) ) { if (autoScroll) { scrollToTarget(); @@ -105,6 +108,7 @@ const ScrollToHint = ({ const iconName = useMemo(() => { const currentScroll = scrollContainer.current?.scrollTop; + // eslint-disable-next-line react-hooks/refs const elementTop = scrollTargetRef?.current?.getBoundingClientRect().top; return elementTop !== undefined && currentScroll !== undefined && elementTop > currentScroll