After topic list is updated, only restore focus to it, if it was
focused before. This avoids jumping focus from say compose box
to topic search unexpectedly when the topic list is updated.
This reduces the complexity of our dependency graph.
It also makes sub_store.get parallel to message_store.get.
For both you pass in the relevant id to get the
full validated object.
This breaks an indirect dependency of stream_data
on the channel module.
It's a verbatim code move, apart from the one-line
helper `has_history_for`. It's not totally clear
to me why the original code doesn't call into
`is_complete_for_stream_id` to early-exit, but
figuring that out is outside the scope of my
change.
It's possible that we will eventually just subsume
this tiny module into topic_list once we finish
breaking all dependencies, but we may want to
reuse this for something like Recent Topics
or other similar UIs.
It's also possible that we'll want to rename
stream_topic_history -> stream_topic_history_data
sometime soon, possibly after we clean up its
dependency on message_util soon.
Instead of prohibiting ‘return undefined’ (#8669), we require that a
function must return an explicit value always or never. This prevents
you from forgetting to return a value in some cases. It will also be
important for TypeScript, which distinguishes between undefined and
void.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
ES and TypeScript modules are strict by default and don’t need this
directive. ESLint will remind us to add it to new CommonJS files and
remove it from ES and TypeScript modules.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Prettier would do this anyway, but it’s separated out for a more
reviewable diff. Generated by ESLint.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
`stream_topic_history` is a more appropriate name as this
module will contain information about last message of a
stream in upcoming commits. Function and variable names
are changed accordingly like:
* topic_history() -> per_stream_history()
* get_recent_names() -> get_recent_topic_names()
* name -> topic_name
We avoid complicated code to update unread counts
by just using vdom.js.
One small change here is that if click on "more
topics", we replace it with the spinner instead
of putting the spinner after it. This saves us
a redraw under the new scheme.
Babel strict generates more code for [...x] than you’d like, while
Babel loose mode assumes x is an array.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This function will make more sense when we start
tracking api calls that retrieve topic history.
The unit tests here are kinda duplicating what we
have in the stream_data tests. If we move the
function out of stream_data, we can kill off the
tests there, but for now I think a bit of duplicate
testing is fine here.
This is mostly for tactical reasons. It's hard to
get 100% test coverage on topic_list.js, but it
should be easy to get 100% test coverage on this
very important function.
I considered just moving this code into topic_data.js,
but it just didn't feel quite right. I feel like
this is a pretty core piece of code that's nice
to be by itself and not be near other complicated
code that does stuff like build widgets or talk
to servers. (And, again, it's not just the actual
code here, which is pretty small, it's the unit
tests, which are inherently verbose to exercise
all the edge cases.)
There was an edge case with the old
code when you had exactly between 6 and 8
topics and all in cache, with a couple of
the topics being unread.
We would show "more topics" when you were
actually seeing all your possible topics.
To test this:
- create 7 topics on Venice
- as Iago, narrow to any of the Venice
topics
- as Aaron, send unreads to 3 or 4
of the other topics
Eventually Iago will have all possible
topics in the sidebar. On master we'll
show "more topics", whereas after this commit
we correctly avoid that.
It's a pretty harmless bug, since it just
leads to a useless zoom-in.
I have always felt we should zoom-in
regardless of how many topics you have,
just for consistency sake, but I also
understand the rationale behind our
current intentions.
This is basically trying to confine the
rendering logic to a smaller function,
since I want to work toward a better
approach for redrawing the topic list.
Also, since the new function is now
purely data-oriented, it will be a
bit easier to test various edge cases.
If you clicked for no more topics and then the server didn't find any,
we once had code that would say "No more topics" in light gray at the
bottom of the topic list.
The feature appears to have been broken by some detail in the
`self.dom` refactoring. More importantly, it's not clear it's useful
as opposed to clutter.
Since we added the `stream.first_message_id` feature, it's now very
rare for the `more topics` option to appear when there aren't in fact
older topics that could be fetched. In cases where there are not, the
UI is still clear about what's happening -- it shows a loading
indicator and then displays a list of topics that doesn't have
anything new.
So we're removing this feature; we can re-add it without too much
difficulty if user feedback in the future suggests it would be useful
after all.
The only place we ever set active-sub-filter is
right after we build the template, so there is
no reason to have it be a separate step.
(I made a similar fix to pm_list recently, and
this helps set the stage for doing vdom-like
stuff.)
The previous logic was a bit byzantine, making a lot of inferences
based on which conditionals had already been processed that made it
hard to read. This simple function approach promises to be more
readable.
Anders Kaseorg
Aman Agrawal
m-e-l-u-h-a-n
Steve Howell
Hashir Sarwar
Steve Howell
Anders Kaseorg
Tim Abbott