From 6fd27ce892523e1629110438304ed5eaafb7c997 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Fri, 7 Jun 2024 10:39:33 -0700 Subject: [PATCH] list_widget: Reduce TypeScript wreckage. Signed-off-by: Anders Kaseorg --- web/src/list_widget.ts | 41 +++++++++++++++-------------------- web/tests/lib/index.js | 1 + web/tests/list_widget.test.js | 15 +++++++------ 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/web/src/list_widget.ts b/web/src/list_widget.ts index 45005dacc6..6e9ec34acd 100644 --- a/web/src/list_widget.ts +++ b/web/src/list_widget.ts @@ -18,7 +18,7 @@ type ListWidgetMeta = { filtered_list: Item[]; reverse_mode: boolean; $scroll_container: JQuery; - $scroll_listening_element: JQuery | JQuery; + $scroll_listening_element: JQuery; }; // This type ensures the mutually exclusive nature of the predicate and filterer options. @@ -46,7 +46,7 @@ type ListWidgetOpts = { callback_after_render?: () => void; post_scroll__pre_render_callback?: () => void; get_min_load_count?: (rendered_count: number, load_count: number) => number; - is_scroll_position_for_render?: (scroll_container: HTMLElement) => boolean; + is_scroll_position_for_render?: () => boolean; filter?: ListWidgetFilterOpts; multiselect?: { selected_items: Key[]; @@ -262,7 +262,7 @@ export function create( old_widget.clear_event_handlers(); } - let $scroll_listening_element: JQuery | JQuery = opts.$simplebar_container; + let $scroll_listening_element: JQuery = opts.$simplebar_container; if ($scroll_listening_element.is("html")) { // When `$scroll_container` is the entire page (`html`), // scroll events are fired on `window/document`, so we need to @@ -430,23 +430,23 @@ export function create( set_up_event_handlers() { // on scroll of the nearest scrolling container, if it hits the bottom // of the container then fetch a new block of items and render them. - meta.$scroll_listening_element.on( - "scroll.list_widget_container", - function (this: HTMLElement) { - if (opts.post_scroll__pre_render_callback) { - opts.post_scroll__pre_render_callback(); - } + meta.$scroll_listening_element.on("scroll.list_widget_container", function () { + if (opts.post_scroll__pre_render_callback) { + opts.post_scroll__pre_render_callback(); + } - if (opts.is_scroll_position_for_render === undefined) { - opts.is_scroll_position_for_render = is_scroll_position_for_render; - } + let should_render; + if (opts.is_scroll_position_for_render === undefined) { + assert(!(this instanceof Window)); + should_render = is_scroll_position_for_render(this); + } else { + should_render = opts.is_scroll_position_for_render(); + } - const should_render = opts.is_scroll_position_for_render(this); - if (should_render) { - widget.render(); - } - }, - ); + if (should_render) { + widget.render(); + } + }); if (opts.$parent_container) { opts.$parent_container.on( @@ -466,11 +466,6 @@ export function create( }, clear_event_handlers() { - // Since `$scroll_listening_element` is of type `JQuery | JQuery` instead - // of just `JQuery`, Typescript is expecting `off` to be called on - // TypeEventHandlers which is confusing. - // - // @ts-expect-error Maybe JQuery.TypeEventHandlers is not defined? meta.$scroll_listening_element.off("scroll.list_widget_container"); if (opts.$parent_container) { diff --git a/web/tests/lib/index.js b/web/tests/lib/index.js index 262bf37096..0a5bc1da35 100644 --- a/web/tests/lib/index.js +++ b/web/tests/lib/index.js @@ -24,6 +24,7 @@ const dom = new JSDOM("", {url: "http://zulip.zulipdev.com/"}); global.DOMParser = dom.window.DOMParser; global.HTMLAnchorElement = dom.window.HTMLAnchorElement; global.HTMLElement = dom.window.HTMLElement; +global.Window = dom.window.Window; global.navigator = { userAgent: "node.js", }; diff --git a/web/tests/list_widget.test.js b/web/tests/list_widget.test.js index 8e805d6d29..0fc4ffd20b 100644 --- a/web/tests/list_widget.test.js +++ b/web/tests/list_widget.test.js @@ -60,6 +60,7 @@ function make_container() { function make_scroll_container() { const $scroll_container = {}; + $scroll_container[0] = {}; $scroll_container.cleared = false; @@ -68,7 +69,7 @@ function make_scroll_container() { $scroll_container.on = (ev, f) => { assert.equal(ev, "scroll.list_widget_container"); $scroll_container.call_scroll = () => { - f.call($scroll_container); + f.call($scroll_container[0]); }; }; @@ -171,9 +172,9 @@ run_test("scrolling", () => { assert.equal(get_scroll_element_called, true); // Set up our fake geometry so it forces a scroll action. - $scroll_container.scrollTop = 180; - $scroll_container.clientHeight = 100; - $scroll_container.scrollHeight = 260; + $scroll_container[0].scrollTop = 180; + $scroll_container[0].clientHeight = 100; + $scroll_container[0].scrollHeight = 260; // Scrolling gets the next two elements from the list into // our widget. @@ -223,9 +224,9 @@ run_test("not_scrolling", () => { assert.equal(get_scroll_element_called, true); // Set up our fake geometry. - $scroll_container.scrollTop = 180; - $scroll_container.clientHeight = 100; - $scroll_container.scrollHeight = 260; + $scroll_container[0].scrollTop = 180; + $scroll_container[0].clientHeight = 100; + $scroll_container[0].scrollHeight = 260; // Since `should_render` is always false, no elements will be // added regardless of scrolling.