This is preparation for enabling an eslint indentation configuration.
90% of these changes are just fixes for indentation errors that have
snuck into the codebase over the years; the others are more
significant reformatting to make eslint happy (that are not otherwise
actually improvements).
The one area that we do not attempt to work on here is the
"switch/case" indentation.
We don't need to special-case the stream cog handler when we
handle the click event for the surrounding header. The browser
will fire the event for the cog first, which stops propagation.
The new list_cursor class is more generic and saves the state
of your cursor across redraws.
Note that we no longer cycle from bottom to top or vice versa.
The node test code that was removed here was kind of complex
and didn't actually assert useful things after calling methods.
This introduces a generic class called list_cursor to handle the
main details of navigating the buddy list and wires it into
activity.js. It replaces some fairly complicated code that
was coupled to stream_list and used lots of jQuery.
The new code interacts with the buddy_list API instead of jQuery
directly. It also persists the key across redraws, so we don't
lose our place when a focus ping happens or we type more characters.
Note that we no longer cycle to the top when we hit the bottom, or
vice versa. Cycling can be kind of an anti-feature when you want to
just lay on the arrow keys until they hit the end.
The changes to stream_list.js here do not affect the left sidebar;
they only remove code that was used for the right sidebar.
We consistently either pass a `then_select_id` into narrow.activate,
or were using the select_first_unread option. Now, we just compute
select_first_unread based on the value of then_select_id.
When in the stream-searchbar, a user can now use the arrow keys to iterate
through the suggestions. Therefore the currently selected list element is
assigned a CSS class 'highlighted_user'.
The main functional testing is done with casper but node test are still
included to keep the high coverage.
Line-wrapping issues are resolved. Night-mode CSS handling is included.
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.
When we activate a stream (or one of its topics), we now scroll
the sidebar so that the stream comes into view. We scroll it
just enough to get it to the top or the bottom, depending on
where it had been offscreen before.
Despite the length of this commit, it is a very straightforward
moving of code from narrow.js -> narrow_state.js, and then
everything else is just s/narrow.foo()/narrow_state.foo()/
(with a few tiny cleanups to remove some code duplication
in certain callers).
The only new functions are simple setter/getters that
encapsulate the current_filter variable:
narrow_state.reset_current_filter()
narrow_state.set_current_filter()
narrow_state.get_current_filter()
We removed narrow.predicate() as part of this, since it was dead
code.
Also, we removed the shim for narrow_state.set_compose_defaults(),
and since that was the last shim, we removed shim.js from the app.
Before this change, we would move "dormant" streams to the bottom
of your stream sidebar, but only if you had 40+ streams.
Now we do this in all cases to be more consistent.
This commit also changes the redraw strategy when we remove rows.
Before this change, we were doing incremental updates, but now we
call build_stream_list to do a complete rebuild. This was partly
motivated by adding the new divider, which would have complicated
the incrememental approach when you removed the last remaining
dormant stream.
The <hr> is supposed to separate the pinned streams from the unpinned
streams, so if the <hr> is the first element (checked by doing
$hr.prev().length === 0), then it means there are no longer any pinned
streams and therefore it isn’t necessary to have a divider.
Fixes: #4395.
The pinned streams were sorted in alphabetic order (i.e. Verona appears
before devel). The reason is that after we plucked pinned streams out from
stream_data.subscribed_streams(), we didn't sort them again, so they
remained in the alphabetic order used in stream_data.
However, we did sort unpinned streams explicitly by using custom compare
function in stream_list.js (by default sort by lowercase stream name,
but when there are more than 40 subscribed streams, sort active streams
first). That's why this issue only relates to pinned streams.
Changes were made to sort pinned streams by lowercase stream name, always,
whether they are active or not (different from unpinned streams).
Tests were added to ensure this overall sort order is correct, i.e.
1. pinned streams are always sorted by lowercase stream name.
2. pinned streams are always before unpinned streams.
3. unpinned streams are sorted by lowercase stream name, if there are more
than 40 subscribed streams, sort active streams at the top, among active
and inactive streams, still sorted by lowercase stream name.
Fixes#3701
In a96fdd18b1, I introduced a few
regressions related to the blue highlighting that happens
in the top left corner for Home, Private messages, Starred
messages, and @-mentions. Basically, we weren't clearing
the highlighting when we thought we were, so Home would stay
blue too long and the other filters wouldn't turn blue.
We went a surprising long time before noticing the regression.
This fix adds a function called deselect_top_left_corner_items()
to clear the blue backgrounds, so that will happen more explicitly.
And then I restored a line of code to pm_list.js that puts the
blue in place when you are in an is:private narrow (vs. a
specific PM narrow).
The fix works by having build_stream_sidebar_row()
automatically update its own unread count when we
build a sidebar row. Currently we rebuild sidebar
rows when we pin/unpin rows.
As an aside, we currently don't really need to rebuild
the sidebar row when we pin, since we're only moving
the DOM, not altering it. But this may change in the
future, so I decided to leave that code path in place.
We may decide to do things in the future like showing
pinned streams with bolder fonts or special icons or
whatever.
Fixes#2902
Some of the work here was done Tomasz Kolek.
When we click on "more conversations" in "Private Messages,"
we call it being "zoomed in." Before this change, when
new PMs arrived, we would rebuild the list and zoom out
again. Now we track the zoomed_in state with a variable.
Also, if you are zoomed in and switch from one PM narrow
to another, we also keep you zoomed in.
This fix also removes some extraneous/redundant code.
Fixes: #2561
I forgot to remove this code in a recent refactoring that copied
this code into activity.js. It should not have caused any errors,
but it's no longer needed.
From subs.js we don't redundantly try to remove an element
from ths sidebar; we just trigger the event.
In stream_list.js we continue to remove the element from
the DOM, and we also remove the widget from our internal
Dict of sidebar rows, so that if we re-subscribe, we know
we'll automatically re-build the widget from the template
and the latest data from stream_data.js.
We used to have hacky code where various functions would call
build_stream_sidebar_row() to get a jQuery object, and then they
would attach the jQuery object to the "sub" object from stream_data.js.
Now build_stream_sidebar_row() localizes the hack of attaching
a UI object to the "sub" object to just one function (and we can
clean this up in a follow-up commit).
Also, the UI object is now a JS object that can close on some useful
state information like the stream name and encapsulate how we
toggle the inactive_stream class.
Finally, we don't have build_stream_sidebar_row() needlessly append
list items to $('#stream_filters') when we know that our callers are
going to re-build the list anyway.
The code removed in this commit added pinned streams to a list
of elems, only to have them added again by the next block of code
(but more concisely). Through some strange quirk of appendTo() this
never created user-facing bugs, but you could clearly see in the console
that it was doing double work.
For cases where we are zoomed in to a stream and then go
to a different narrow (Home, PMs, etc.), we now let
topic_list.zoom_out orchestrate the removal of the topic list
instead of stream_list.zoom_out.
This will help us when we move to a world where topic_list
redraws topic lists on zoom-in/zoom-out, because we won't
waste effort rebuilding lists that are about to be removed.
The flow for topic list zooming is kind of complicated now, but
it's mostly a consequence of the way the UI works.
* stream_list tells topic_list to set up the topic list
click handlers to have callbacks to stream_list
* topic_list click handlers call to stream_list zoom methods
to hide/show all the other streams
* stream_list zoom methods call back to topic_list methods to
redraw topics as needed (this isn't happening yet, but allowing
topic_list.js to know that it's zoomed will set the stage for
this to happen in a more controlled manner)
We no longer use active_stream_name(), which was mostly a
duplicate of narrow.stream(). For nonsensical queries like
"stream:foo stream:bar" the behavior may change slightly here.
We know that we don't handle non-sensical queries particularly
well, but at least if we always go through narrow.stream(),
the behavior will be consistent.
I did test this with some sensible compound narrows, like searching
for a keyword within a stream.
We now use stream_id as our key to rename streams, which
should prevent a few race conditions long term. (We are
still possibly contending with other events that use
stream_name as a key, so this is not perfect.)
This change makes most of the logic on set_count() live
on our per-stream topic list widget. We can find the
jQuery object directly now rather than using the
complicated iterate_to_find() method.
This also brought along:
iterate_to_find (copied, see large comment explaining why)
activate_topic (extracted from a one-liner)
set_count (formerly stream_list.set_subject_count)
For get_topic_filter_li, we now pass in stream_li instead of
stream to decouple parent/child responsibilities between the
components.
Also, I made some s/subject/topic/ fixes.
This creates the new topic_list.js module, and the first
function that we extract is topic_list.update_count_in_dom().
This function needed to be decoupled from some non-topic-list
stuff which was overly complicated.
I make server_events slimmer by not handling a specific
property when subs.update_subscription_properties() should
do all the dispatching (and mostly did).
And then since update_subscription_properties() has
a "sub" already, I can call directly to stream_list code
and remove a function from subs.js. Since I lose the
wrapper function in subs.js, I rename the stream_list
function as part of this commit.
The only code that gets slightly heavier here is that
we have two lines in the 'pin_to_top' case instead of one.
The startup code in subs.js used to intermingle data
stuff and UI stuff in a loop inside a called function,
which made the code hard to reason about.
Now there is a clear separation of concerns, with these methods
being called in succession:
stream_data.initialize_from_page_params();
stream_list.create_initial_sidebar_rows();
The first method was mostly extracted from subs.js, but I simplified
some things, like not needing to make a copy of the hashes
we were passed in, plus I now garbage collect email_dict. Also,
the code path that initialize_from_page_params() mostly replaces
used to call create_sub(), which fired a trigger, but now it
just does data stuff.
Once the data structure is built up, it's a very simple matter
to build the initial sidebar rows, and that's what the second
method does.
This replaces add_stream_to_sidebar(), which was kind of a misleading
name, and it also adds a couple lines of code that were always
called right after calling add_stream_to_sidebar().
Assigns hotkey 'w' to search streams.
Only show search box when active. Activate with hotkey or by clicking
STREAMS.
Filter matches at the beginning of words in stream name.
Behaviour is otherwise almost identical to user search.
Casper tests.
Like the Stream Subject lists, Private messages are now shown
when the user clicks on the "Private message" link. User can drill in
to get more than 5 conversations. Selecting PMs from the user or group
PM lists on the right sidebar also opens the list & highlights the
selected conversation.
[Edited by tabbott@mit.edu to fix some small bugs.]
Before this change, we were using sequentially generated ids
on the client side to identify streams. Now we just use
the ids from the server. The goal here is to reduce the
confusion of having two different ids attached to a stream.
Also, not that it matters a ton, but this also means that
the browser basically has an immutable id for each stream
that is future-proof to reloads, multiple create_sub calls, etc.
It also a bit easier to grep for ".stream_id" than ".id".
(imported from commit 057f9e50dfee127edfe3facd52da93108241666a)
This function can redraws the lock icon (or lack of lock icon)
for a stream in the stream sidebar. It can be called when
admins change the stream privacy.
(imported from commit 880133d02525137094c48ecad8cf2dfff59f3307)
The new name is more descriptive of what the function does, and it
also has the side benefit of cleaning up greps for the backend
function called build_narrow_filter().
(imported from commit 4b88fa863d7c1751946c78977f2ffaf19dd3ae5e)
The type parameter was always passed in as "stream", and we
only render the stream_sidebar_row template, so let's not
pretend like we support arbitrary message types here.
(imported from commit 8a852a68ddda336024793f6fdafa648883bb815e)
We now call add_stream_to_sidebar() instead. The old function was
only ever called for streams, so the newly named function is more
descriptive of what it does.
(imported from commit 7ae373279ea9987d3637cdbdc427680ac989fe86)
This is a node test that verifies that
stream_list.add_stream_to_sidebar() creates the right
DOM when it renders the stream_sidebar_row template.
The test also makes sure that the DOM gets put in the
correct place to be retrieved by stream_list.get_stream_li()
calls.
(imported from commit ed4c0148da2261870e3db5a9b553913788b4eccd)
In the first cut at topic zoom, I was re-rendering the
streams list, but this created glitches with orphaned
list items. The reproducible bug was that unread counts
on unshown streams weren't updating.
In the new approach, I keep the elements more permanent, and
I just hide and show them as needed, either through jQuery
show/hide or permanent CSS selectors.
I got rid of toggle_zoom(), so that we just explicitly zoom
in and zoom out in all situations. In particular, when we
narrow, it's more clear now that only stay zoomed in when
we're narrowing to the same stream as before (including topic
narrows within that stream).
When you zoom in, the number of topics is no longer limited
to 30, since that was kind of arbitrary anyway. (In practice,
the number of topics is usually well under 30, anyway, due to
the way we track them on the client.)
(imported from commit 5b6c143dee9ba9fe557d8cc36335ff28efb4b0de)
This link lets you zoom in to more topics. We only show it if
there are topics that we had to hide to respect the max-5 limit
along with other rules of when you show topics.
This is feature flagged to staging only.
(imported from commit 9915004ec2eb3df7416fe45c0e60cebcd7fecfea)
When your left sidebar is zoomed to show just one stream,
there is a link to to show all streams again.
(imported from commit 92f39b042168c443cbb9f524bf892557ef492551)
If you double-click on a stream that you've narrowed to, it
will either zoom in or zoom out the left sidebar view. Zooming
in shows just that stream; zooming out shows all streams.
This is feature flagged to staging only.
(imported from commit 6fdb3cacd68635f313f2a8a81edf2d6101cce2cb)
This commit doesn't actually add the final UI to zoom/unzoom
topics, because I want to keep those in separate commits, in
case we change how to enable the feature. But this commit
adds a toggle_zoom() function that zooms/unzooms topics.
Zooming is minimally invasive, because we don't really introduce
many extra elements to the UI; instead, we just make the list of
streams be a list of length one (i.e. the active stream). This
gives us a lot of stuff for free, basically, like unread counts, etc.
(imported from commit 814c1361b6210d1591b4174bed1d6e0c98a3f255)
Shubham Dhama
Tim Abbott
Steve Howell
gooca
Florian Jüngermann
Logan Williams
YJDave
Cynthia Lin
Greg Price
Rohitt Vashishtha
Steve Howell
Cory Lynch
Harshit Bansal
Brock Whittaker
Raghav Jajodia
chao1995
Elliott Jin
KingxBanana
Rafid Aslam
lonerz
Tommy Ip
Kartik Maji
Michael Cordover
Tim Abbott
Vishnu Ks
Kara McNair
Allen Rabinovich
Steve Howell
Tim Abbott