The data attribute here has some value if you're
inspecting the HTML in the browser, but it's not
worth the extra code.
All the list items have data-stream-id, so there's
no need for the parent to have it.
This fixes the most core data structures inside of
muting.js. We still use stream names for incoming
data to set_muted_topics and outgoing data from
get_muted_topics.
This will make us more resilient to stream name changes.
Before, if you were logged on when a stream rename
occured, topics that were muted under that stream would
appear to be unmuted. (You could fix it with a reload,
but it can be jarring to have a bunch of unread messages
appear in your feed suddenly.)
Fixes#11033
This small modules nicely breaks down the
responsibilities of topic_list and stream_list
when it comes to zooming in and out of topics
(also known as hitting "more topics" or "All
Streams).
Before this, neither module was clearly in
charge, and there were kind of complicated
callback mechanisms. The stream_list code
was asking topic_list to create click handlers
that called back into stream_list.
Now we just topic_zoom set up its own click
handlers and delegate out to the other two
modules.
This allows several modules to no longer need
to import `narrow` (or, in our current pre-import
world, to not have to use that global).
The broken dependencies are reflected in the node
tests, which should now run slightly faster.
This commit prepares the frontend code to be consumed by webpack.
It is a hack: In theory, modules should be declaring and importing the
modules they depend on and the globals they expose directly.
However, that requires significant per-module work, which we don't
really want to block moving our toolchain to webpack on.
So we expose the modules by setting window.varName = varName; as
needed in the js files.
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 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.
We want to scroll the left sidebar to the top as soon as the user
zooms in on a stream, and we don't want to wait for the server,
otherwise we'll get jumpiness.
This commit is a bit complicated, because we do full redraws of
the topic list frequently, and we don't want to randomly obliterate
our "No more topics found" message, so we need to keep a bit of
extra state around.
We now use a template to render the "more topics" link.
We also remove an unnecessary conditional and an unnecessary
attribute.
Finally, our unit tests are a bit more granular now.
This will make testing a bit easier (we can stub stuff before
building the widget), and it will eventually give us more control
on redrawing the topic list.
We were parameterizing max_topics, but it made the calling sequences
unnecessarily complicated. We don't ever override the value, not
even in tests, so now we just set in build_list().
This puts build_list on the widget object, which will make it a
bit easier to unit test, and it's more consistent with the rest of
the function. This also reduces the scope of the `my_stream_name`
variable and moves the initialization of `self.topic_items` into
build_list.
If a user re-narrows to another stream before our server gives
us more topic history, or they zoom out, we can avoid drawing
the topic list. Note that our data structures will still be
updated, although the only time that really matters is for
the corner case of a low-traffic stream. For a low traffic
stream that only had 3 or 4 topics in the original message
fetch, but has longer history, the next time you open the
stream in the sidebar, even when you're zoomed out, you will
see more topics.
Despite a few warts, we are going forward with getting topic
history from the server when you click "more topics." This
commit simplifies the code by removing the feature flag
checks.
Variable `show_topic` was assigned in both branches of the
conditional, but the assignment in the "then" branch was useless,
since the variable wasn't subsequently read. Hence the assignment can
be dropped, leaving the "then" branch empty. The "if" statement can
then be simplified by removing the "then" branch entirely and flipping
the condition. Since `show_topic` is now only used inside the "if", it
is slightly tidier (though semantically equivalent) to move its
declaration inside.
Until we have an easy way to consistently determine whether a
stream has more topics than have been loaded already, we err
on the side of showing a "more topics" link. This in some ways
leads to a more consistent experience where you can zoom in on
any stream, even one that's really new.
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.
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.
This new module tracks the recent topic names for any given
stream.
The code was pulled over almost verbatim from stream_data.js,
with minor renames to the function names.
We introduced a minor one-line function called stream_has_topics.
We now have all of our callers into recent_topics code just
receive a list of topic names from get_recent_topic_names().
This is more encapsulated than handing off tiny little
structures to the three callers, two of whom immediately
mapped the objects to names, and one of whom needlessly
used the now defunct name canon_subject field.
The consolidation here removes some "subject" references, and
now all lookup are by stream id, not stream name.
The diff here is a bit daunting, but it's mostly simplification
of tests and calling code. Two of the callers now need to look
up stream ids, but they are otherwise streamlined.
The main change here is to stream_data.js, and we replace the
`canon_subject` and `subject` fields with `name`.
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.
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.
Steve Howell
Armaan Ahluwalia
Shubham Dhama
Tim Abbott
gooca
Brock Whittaker
Max Schaefer
Steve Howell
Rohitt Vashishtha