widgets: Cache event handler instead of entire jquery for each widget.

In 61b3c698af we stopped restoring widgets
from the cache, so the only purpose of the cache remaining was to access
the event handler patched onto the widget jquery.

Now instead of the entire jquery object, we cache just the event handler
since that's the only thing we need from the cache, and do not patch it
onto the widget jquery object.
This commit is contained in:
N-Shar-ma 2024-05-08 19:46:22 +05:30 committed by Tim Abbott
parent cc793612f0
commit cb58752909
6 changed files with 37 additions and 23 deletions

View File

@ -42,7 +42,7 @@ export function activate({
) => void;
extra_data: PollWidgetExtraData;
message: Message;
}): void {
}): (events: Event[]) => void {
const is_my_poll = people.is_my_user_id(message.sender_id);
const poll_data = new PollData({
message_sender_id: message.sender_id,
@ -237,7 +237,7 @@ export function activate({
});
}
$elem.handle_events = function (events: Event[]) {
const handle_events = function (events: Event[]): void {
for (const event of events) {
poll_data.handle_event(event.sender_id, event.data);
}
@ -249,4 +249,6 @@ export function activate({
build_widget();
render_question();
render_results();
return handle_events;
}

View File

@ -89,7 +89,7 @@ export function get_message_events(message: Message): SubmessageEvents | undefin
}
export function process_widget_rows_in_list(list: MessageList | undefined): void {
for (const message_id of widgetize.widget_contents.keys()) {
for (const message_id of widgetize.widget_event_handlers.keys()) {
if (list?.get(message_id) !== undefined) {
process_submessages({message_id, $row: list.get_row(message_id)});
}

View File

@ -224,7 +224,7 @@ export function activate({$elem, callback, extra_data, message}) {
const parse_result = todo_widget_extra_data_schema.safeParse(extra_data);
if (!parse_result.success) {
blueslip.warn("invalid todo extra data", parse_result.error.issues);
return;
return () => {};
}
const {data} = parse_result;
const {task_list_title = "", tasks = []} = data || {};
@ -383,7 +383,7 @@ export function activate({$elem, callback, extra_data, message}) {
});
}
$elem.handle_events = function (events) {
const handle_events = function (events) {
for (const event of events) {
task_data.handle_event(event.sender_id, event.data);
}
@ -395,4 +395,6 @@ export function activate({$elem, callback, extra_data, message}) {
build_widget();
render_task_list_title();
render_results();
return handle_events;
}

View File

@ -35,14 +35,14 @@ type WidgetValue = Record<string, unknown> & {
callback: (data: string) => void;
message: Message;
extra_data: WidgetExtraData;
}) => void;
}) => (events: Event[]) => void;
};
export const widgets = new Map<string, WidgetValue>();
export const widget_contents = new Map<number, JQuery>();
export const widget_event_handlers = new Map<number, (events: Event[]) => void>();
export function clear_for_testing(): void {
widget_contents.clear();
widget_event_handlers.clear();
}
function set_widget_in_message($row: JQuery, $widget_elem: JQuery): void {
@ -82,28 +82,28 @@ export function activate(in_opts: WidgetOptions): void {
// the HTML that will eventually go in this div.
const $widget_elem = $("<div>").addClass("widget-content");
widgets.get(widget_type)!.activate({
const event_handler = widgets.get(widget_type)!.activate({
$elem: $widget_elem,
callback,
message,
extra_data,
});
widget_contents.set(message.id, $widget_elem);
widget_event_handlers.set(message.id, event_handler);
set_widget_in_message($row, $widget_elem);
// Replay any events that already happened. (This is common
// when you narrow to a message after other users have already
// interacted with it.)
if (events.length > 0) {
$widget_elem.handle_events(events);
event_handler(events);
}
}
export function handle_event(widget_event: Event & {message_id: number}): void {
const $widget_elem = widget_contents.get(widget_event.message_id);
const event_handler = widget_event_handlers.get(widget_event.message_id);
if (!$widget_elem) {
if (!event_handler || message_lists.current?.get_row(widget_event.message_id).length === 0) {
// It is common for submessage events to arrive on
// messages that we don't yet have in view. We
// just ignore them completely here.
@ -112,5 +112,5 @@ export function handle_event(widget_event: Event & {message_id: number}): void {
const events = [widget_event];
$widget_elem.handle_events(events);
event_handler(events);
}

View File

@ -251,7 +251,7 @@ run_test("activate another person poll", ({mock_template}) => {
set_widget_find_result("button.poll-question-remove");
set_widget_find_result("input.poll-question");
poll_widget.activate(opts);
const handle_events = poll_widget.activate(opts);
assert.ok($poll_option_container.visible());
assert.ok($poll_question_header.visible());
@ -298,7 +298,7 @@ run_test("activate another person poll", ({mock_template}) => {
},
];
$widget_elem.handle_events(vote_events);
handle_events(vote_events);
{
/* Testing data sent to server on voting */
@ -318,7 +318,7 @@ run_test("activate another person poll", ({mock_template}) => {
},
];
$widget_elem.handle_events(add_question_event);
handle_events(add_question_event);
});
run_test("activate own poll", ({mock_template}) => {

View File

@ -36,6 +36,7 @@ const sample_events = [
let events;
let $widget_elem;
let handle_events;
let is_event_handled;
let is_widget_activated;
@ -44,11 +45,12 @@ const fake_poll_widget = {
is_widget_activated = true;
$widget_elem = data.$elem;
assert.ok($widget_elem.hasClass("widget-content"));
$widget_elem.handle_events = (e) => {
handle_events = (e) => {
is_event_handled = true;
assert.deepStrictEqual(e, events);
};
data.callback("test_data");
return handle_events;
},
};
@ -66,6 +68,7 @@ function test(label, f) {
run_test(label, ({override}) => {
events = [...sample_events];
$widget_elem = undefined;
handle_events = undefined;
is_event_handled = false;
is_widget_activated = false;
widgetize.clear_for_testing();
@ -73,10 +76,11 @@ function test(label, f) {
});
}
test("activate", () => {
test("activate", ({override}) => {
// Both widgetize.activate and widgetize.handle_event are tested
// here to use the "caching" of widgets
// here to use the "caching" of widget event handlers.
const $row = $.create("<stub message row>");
$row.length = 1;
$row.attr("id", `message-row-${message_lists.current.id}-2909`);
const $message_content = $.create(`#message-row-${message_lists.current.id}-2909`);
$row.set_find_results(".message_content", $message_content);
@ -106,14 +110,14 @@ test("activate", () => {
is_widget_elem_inserted = false;
is_widget_activated = false;
is_event_handled = false;
assert.ok(!widgetize.widget_contents.has(opts.message.id));
assert.ok(!widgetize.widget_event_handlers.has(opts.message.id));
widgetize.activate(opts);
assert.ok(is_widget_elem_inserted);
assert.ok(is_widget_activated);
assert.ok(is_event_handled);
assert.equal(widgetize.widget_contents.get(opts.message.id), $widget_elem);
assert.equal(widgetize.widget_event_handlers.get(opts.message.id), handle_events);
message_lists.current = undefined;
is_widget_elem_inserted = false;
@ -146,6 +150,7 @@ test("activate", () => {
assert.ok(!is_event_handled);
/* Testing widgetize.handle_events */
message_lists.current = {id: 2};
const post_activate_event = {
data: {
idx: 1,
@ -154,10 +159,15 @@ test("activate", () => {
message_id: 2001,
sender_id: 102,
};
$widget_elem.handle_events = (e) => {
handle_events = (e) => {
is_event_handled = true;
assert.deepEqual(e, [post_activate_event]);
};
widgetize.widget_event_handlers.set(2001, handle_events);
override(message_lists.current, "get_row", (idx) => {
assert.equal(idx, 2001);
return $row;
});
is_event_handled = false;
widgetize.handle_event(post_activate_event);
assert.ok(is_event_handled);