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 <noreply@anthropic.com>
This commit is contained in:
Dennis Oelkers
2026-03-09 17:42:00 +01:00
committed by GitHub
parent 97a1306243
commit 2f15a2df25
3 changed files with 88 additions and 2 deletions

View File

@@ -0,0 +1,5 @@
type = "fixed"
message = "Fix scrolling to newly created widgets by checking visibility of the correct elements."
issues = ["25237"]
pulls = ["25257"]

View File

@@ -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
* <http://www.mongodb.com/licensing/server-side-public-license>.
*/
import { isElementVisibleInContainer } from './ScrollToHint';
describe('isElementVisibleInContainer', () => {
const mockElement = (rect: Partial<DOMRect>): 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);
});
});

View File

@@ -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