Extracting this helper library will help us avoid an import loop
between notifications.py and message.py (with bugdown in between).
But in addition to that, it's a more natural model, since some of the
uses for these functions weren't part of the notifications code
anyway.
I often find myself looking manually through the reflog of `master` to
find a commit I previously reset to with tools/reset-to-pull-request .
Sometimes I want to see a previous version of a PR I'm reviewing a
revised version of; sometimes to look at two related PRs together.
So, here's a feature to automate that by saving each PR branch in its
own ref, with a name like `refs/remotes/pr/1234` -- or `pr/1234`, as
you'd normally refer to it.
To enable this, set the new config option:
$ git config zulip.prPseudoRemote pr
(Or you can pick another name.)
The reason I hesitate to just make this the behavior for everyone
immediately is that the resulting `pr/1234` refs will naturally
accumulate and may clutter up the view -- and because with the
`refs/remotes/` style of name I've chosen, it requires a bit of
Git plumbing to clean them up. (Use `git update-ref -d`.)
I'll play with it and iterate; comments welcome from other willing
early adopters.
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);
},
This function is more cohesive and always takes in
a jQuery object containing exactly one DOM element,
and it does all stuff at the jQuery level of
abstraction (no raw DOM).
It's a pretty simple extraction--removing the level
of indentation makes the diff a bit noisy.
We shorten the name of the function and avoid having
all the callers call `.get()`. Now we mostly stay
in jQuery "space", which avoids some confusion about
when we're dealing with raw DOM elements and which
will facilitate unit testing.
Changed search pill padding, `.navbar-search` flex-wrap to match with
the CSS refactoring in 66df4e3e84.
The `height: 100%` changes to `.navbar-search` and `.input-append`
make up for the issue in which the pills overflowed in the mobile
view due to `.navbar-search` height being declared 40px explicitly
while the actual heiight in mobile view was shorter.
It's sorta an unusual state to get into, to have a user own a
deactivated bot, when they can't create a bot of that type, but
definitely a valid possibility that we should be checking for.
Fixes#10087.
Currently on zoom out from stream topics, scrollbar didn't scroll back
to opened stream. Because call to scroll-to-stream func isn't called
after all streams view is displayed. So wrong stream element is
passed to func.
Fix this by calling scroll-to-stream func after all-stream-list view
is displayed.
This is a follow-up in response to Tim's comments on #9951.
In instances where all messages from a BitBucket integration are
grouped under one user specified topic (specified in the URL), we
should include the title of the PR in the message body, since
the availability of a user-specified topic precludes us from
including it in the topic itself (which was the default behaviour).
This is a follow-up in response to Tim's comments on #9951.
In instances where all messages from a Gogs integration are
grouped under one user specified topic (specified in the URL), we
should include the title of the PR in the message body, since
the availability of a user-specified topic precludes us from
including it in the topic itself (which was the default behaviour).