From 1ffc84d947ffa1280c3f5011ec99b216e640d53d Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Sun, 7 Jun 2026 12:32:39 +0800 Subject: [PATCH] implement the IfNeeded part of scrollIntoViewIfNeeded --- src/browser/tests/element/position.html | 17 +++++++++---- src/browser/tests/window/scroll_dedup.html | 28 ++++++++++++++++++++++ src/browser/webapi/Element.zig | 6 +++++ src/browser/webapi/Window.zig | 28 ++++++++++++++-------- 4 files changed, 64 insertions(+), 15 deletions(-) create mode 100644 src/browser/tests/window/scroll_dedup.html diff --git a/src/browser/tests/element/position.html b/src/browser/tests/element/position.html index 9f584cb8..1f9f0319 100644 --- a/src/browser/tests/element/position.html +++ b/src/browser/tests/element/position.html @@ -125,14 +125,21 @@ const targetY = deep.getBoundingClientRect().y; testing.expectTrue(targetY > 0); - window.scrollTo(0, 0); - testing.expectEqual(0, window.scrollY); - + // scrollIntoView() always brings the element's top to the scroll offset. + window.scrollTo(0, 999999); deep.scrollIntoView(); testing.expectEqual(targetY, window.scrollY); - // scrollIntoViewIfNeeded behaves the same here (brings the element in view). - window.scrollTo(0, 0); + // scrollIntoViewIfNeeded() is a no-op when the element is already within the + // viewport band [scrollY, scrollY + innerHeight]. Sit just below the + // element's top so a stray scroll would visibly move scrollY. + const inView = Math.max(0, targetY - 1); + window.scrollTo(0, inView); + deep.scrollIntoViewIfNeeded(); + testing.expectEqual(inView, window.scrollY); + + // ...but it does scroll when the element is outside the band. + window.scrollTo(0, targetY + window.innerHeight + 100); deep.scrollIntoViewIfNeeded(); testing.expectEqual(targetY, window.scrollY); } diff --git a/src/browser/tests/window/scroll_dedup.html b/src/browser/tests/window/scroll_dedup.html new file mode 100644 index 00000000..a9a1552c --- /dev/null +++ b/src/browser/tests/window/scroll_dedup.html @@ -0,0 +1,28 @@ + + + + + + diff --git a/src/browser/webapi/Element.zig b/src/browser/webapi/Element.zig index 1c1077d4..337264d8 100644 --- a/src/browser/webapi/Element.zig +++ b/src/browser/webapi/Element.zig @@ -1475,6 +1475,12 @@ pub fn clone(self: *Element, deep: bool, frame: *Frame) !*Node { pub fn scrollIntoViewIfNeeded(self: *Element, center_if_needed: ?bool, frame: *Frame) void { _ = center_if_needed; + const y = calculateDocumentPosition(self.asNode()); + const scroll_y: f64 = @floatFromInt(frame.window.getScrollY()); + const viewport_height: f64 = @floatFromInt(frame.window.getInnerHeight()); + if (y >= scroll_y and y <= scroll_y + viewport_height) { + return; + } self.scrollIntoView(null, frame); } diff --git a/src/browser/webapi/Window.zig b/src/browser/webapi/Window.zig index c32a7eb1..40db7859 100644 --- a/src/browser/webapi/Window.zig +++ b/src/browser/webapi/Window.zig @@ -54,6 +54,10 @@ const Notification = @import("../../Notification.zig"); const log = lp.log; const IS_DEBUG = builtin.mode == .Debug; +// Faux-layout viewport height. Exposed as window.innerHeight and used to decide +// whether an element is already within view (e.g. scrollIntoViewIfNeeded). +const DEFAULT_INNER_HEIGHT: u32 = 1080; + const Allocator = std.mem.Allocator; const Execution = js.Execution; @@ -700,6 +704,10 @@ pub fn getScrollY(self: *const Window) u32 { return self._scroll_pos.y; } +pub fn getInnerHeight(_: *const Window) u32 { + return DEFAULT_INNER_HEIGHT; +} + const ScrollToOpts = union(enum) { x: i32, opts: Opts, @@ -711,17 +719,17 @@ const ScrollToOpts = union(enum) { }; }; pub fn scrollTo(self: *Window, opts: ScrollToOpts, y: ?i32, frame: *Frame) !void { - switch (opts) { - .x => |x| { - self._scroll_pos.x = @intCast(@max(x, 0)); - self._scroll_pos.y = @intCast(@max(0, y orelse 0)); - }, - .opts => |o| { - self._scroll_pos.x = @intCast(@max(0, o.left)); - self._scroll_pos.y = @intCast(@max(0, o.top)); - }, + const new_x: u32, const new_y: u32 = switch (opts) { + .x => |x| .{@intCast(@max(x, 0)), @intCast(@max(0, y orelse 0))}, + .opts => |o| .{@intCast(@max(0, o.left)), @intCast(@max(0, o.top))}, + }; + + if (new_x == self._scroll_pos.x and new_y == self._scroll_pos.y) { + return; } + self._scroll_pos.x = new_x; + self._scroll_pos.y = new_y; self._scroll_pos.state = .scroll; // We dispatch scroll event asynchronously after 10ms. So we can throttle @@ -1002,7 +1010,7 @@ pub const JsApi = struct { // [Replaceable] (CSSOM-View): writable so assignment overwrites rather than throws. pub const innerWidth = bridge.property(1920, .{ .template = false, .readonly = false }); - pub const innerHeight = bridge.property(1080, .{ .template = false, .readonly = false }); + pub const innerHeight = bridge.property(DEFAULT_INNER_HEIGHT, .{ .template = false, .readonly = false }); pub const devicePixelRatio = bridge.property(1, .{ .template = false, .readonly = false }); pub const opener = bridge.accessor(Window.getOpener, null, .{});