Steve Howell 441a21fac8 Use jQuery objects in MessageListView._rows. ...

The values of this dictionary used to be raw DOM elements,
but get_row() wraps them again, so there's not a huge
reason to store them as raw DOM elements internally.  It
is slightly easier to reason about the code if everything
stays at the jQuery level.

To preserve the old behavior here, we have to do something
that is kind of ugly, but at least it's explicit now.  In
the old code, our cache was DOM elements, and if an id
wasn't in the cache, we would sneakily return $(undefined)
with this code in get_row():

    return $(this._rows[id]);

And it turns out that $(undefined) is basically just a
zero-element jQuery object.  A lot of our code depends
on this behavior and just works around the zero-element
objects as needed with checks like this:

    if (this.selected_row()).length === 0) {
        // don't try to get offset
    }

For now we just preserve this behavior.  We could eventually
be more strict here, or at least have aggressive warnings
on cache misses, but we'd need to retrofit code to be
able to call something like `has_rendered_selection()`
and/or deal with `undefined` as the return value for the case
where the selection hasn't been rendered.

Here is some example code that would cause tracebacks if
we just returned `undefined` for cache misses:

    rerender_preserving_scrolltop: function () {
        // old_offset is the number of pixels between the top of the
        // viewable window and the selected message
        var old_offset;
        var selected_row = this.selected_row();
        var selected_in_view = selected_row.length > 0;
        if (selected_in_view) {
            old_offset = selected_row.offset().top;
        }
        return this.rerender_with_target_scrolltop(selected_row,
            old_offset);
    },

2018-07-27 11:14:53 -07:00

42 KiB

Raw Blame History

View Raw