This reverts commit eb2bdb706 "sidebar: Narrow to latest topic if
not in stream." On a trial deploy, many users were surprised and
preferred the old behavior.
We now narrow to the latest topic in stream if we are narrowing from
outside the stream, and show all topics grouped together (previous
default) if we are already narrowed to the stream.
Fixes#7555.
Here are the functions in top_left_corner:
get_global_filter_li: pure code move
update_count_in_dom: simplifed copy of similar function in stream_list.js
update_dom_with_unread_counts: pure code move, split out from function
of same name in stream_list.js
delselect_top_left_corner_items: pure code move
handle_narrow_activated: pure code move + rename
handle_narrow_deactivated: pure code move, split out from from function
of smae name in stream_list.js
This function was actually de-selecting stream sidebar items
before. Now we just explicitly de-select top-left items in it,
and we do stream-sidebar stuff in update_stream_sidebar_for_narrow().
This fix simplifies how we re-render topic lists when we
re-narrow or zoom out from a topic list.
* The topic_list.zoom_out() no longer gets called as
part of re-narrowing, and we eliminate the clear_topics
option.
* For all situations where we narrow to a filter that does
not have a topic, we simply call the new function
clear_topics().
* The stream_list code no longer calls remove_expanded_topics()
in cases where the new narrow has a topic. This allows us
to optimize away scroll/flicker churn a little more easily.
As part of this, we rename maybe_activate_stream_item() to
update_stream_sidebar_for_narrow(), since the function clears
stuff as well as turning stuff on.
This is mostly a pure code extraction. It makes the call
to reset_to_unnarrowed() happen later in sequence.
The order of operations here is mostly unimportant, but
there may actually be some tiny user-facing benefit
in terms of having the logic happen more sequentially.
BEFORE:
reset streams
fix top left
redraw streams
AFTER:
fix top left
reset streams
redraw streams
If you go into "more topics" for a stream with many topics,
and then scroll down, and then zoom out again with "All
streams", we make sure the active stream is still in view.
We have code that can automatically scroll an element into "view"
in its container. We use this for stream sidebar rows inside the
stream list.
Generally the stream sidebar rows are small enough to fit into
the container, and the prior algorithm worked correctly for that
scenario.
If you have lots of topics, however, and a short screen, the
algorithm was being too aggressive. For example, if the top
wasn't showing, it would scroll the top into view, but at the
cost of scrolling the bottom out of view.
This fix makes the general scrolling algorithm more tame.
Part of the user-facing problem is that the element we pass
into the scrolling code for the stream sidebar rows is bigger
than the part of the row that actually should be shown on
screen. Nevertheless, it makes sense here to make the general
algorithm more robust.
This function no longer sets properties to false, so the supported
way of doing this is to instead use prop(foo, false). Some tests
had to be fixed to accommodate this.
We eliminate `.get(0)` calls in buld_stream_list.
The easy case is that we stop building jQuery objects
for the splitters only to pull out the DOM immediately.
The more subtle case is that we also don't do `.get(0)` calls
to get DOM out of our individual list items. By passing
in full jQuery objects to `append()`, we should prevent ourself
from orphaning the old objects, which may in the future have
things like tooltip logic attached to them.
Now we use a consistent approach to find the list items for
Home/Starred messages/Mentioned in the upper corner.
In particular, we get rid of the complicated
iterate_to_find() function.
Given a stream id, we now find list items using the internal
data structures we created when we built the sidebar, rather than
using a jQuery selector.
This change should lead to clearer tracebacks when our
assumption about the stream list's list items get
violated, and we also short circuit some code in the
caller that tries to scroll to the active stream.
In stream_list.js we have some code to handle narrow activations,
and we were calling unread_ops.process_visible() only for
stream activations, not for PM-related activations, etc., so
our approach was inconsistent.
It also turns out that the call is redundant, since we call
unread_ops.process_visible() when the message pane scrolls as
part of updating the content.
Ideally, we want a more rigorous approach where we make this
call precisely when the new messages become visible to the user,
but the purpose of this fix is to de-clutter the stream_list
logic.
If you were narrowed to an unpinned stream, and then pinned it,
we were mostly redrawing the sidebar correctly, but we weren't
setting the active-filter class. Now we accomplish this by
calling maybe_activate_stream_item(), which also reduces some
code duplication. (The new code introduces a bit of extra logic
to do `stream_li.addClass('active-filter')`.
This commit changes stream_data.in_home_view() to
take a stream_id parameter, which will make it more
robust to stream name changes.
This fixes a bug. Now when an admin renames a stream
you are looking at, it will correctly show itself to
be un-muted. (Even with this fix, though, the stream
appears to be inactive.)
Some callers still do lookups by name, and they will
call name_in_home_view() for now, which we can
hopefully deprecate over time.
Rather than having get_stream_li() look up stream id using
stream name, we force the callers to pass in the stream id.
This adds an extra line to most of the callers for now, but
this will eventually change as we fix some of the callers to
have their callers pass in stream_id.
In places where we now call stream_data.get_stream_id() to
get the stream id, we will be more resilient toward stream
renamings, at least until the next reload, since
stream_data.get_stream_id() can resolve old names that
are stored when we process stream-rename events.
We no longer have get_stream_li() delegate to get_filter_li(),
which simplifies the logic in get_filter_li() and makes
get_stream_li() more direct.
We also move the two functions closer to each other in the file.
The function modals.is_active() can see if modals are open
without having to look at the DOM. This should make it snappier
to type in the compose box. Even if the speedup is pretty minor,
not having to worry about jQuery slowness should make it easier
to diagnose future compose box issues.
The new function gets used in other places, too, where performance
isn't so much an issue.
If we pin a stream, we now scroll up as needed to make sure the
stream is still in view after pinning it. (Note that we don't do
this in the un-pinning case, since users un-pinning stuff may be
doing cleanup on pinned streams they no longer care about.)
Fixes#1714.
Logan Williams
YJDave
Cynthia Lin
Greg Price
Rohitt Vashishtha
Florian Jüngermann
Tim Abbott
Steve Howell
Steve Howell
Cory Lynch
Harshit Bansal