For unknown reasons, 6f7b973d3b
introduced an invalid semi-duplicate call to ui_report.error that
passed the wrong number of parameters.
We should just call `ui_report.error`, which already had logic to
handle the distinction between 40x and 50x errors.
Fixes an issue reported in #18273.
Moved `admin_default_stream_list` template to `/templates/settings/`
folder as earlier, it was inaccurately placed within the `/templates`
folder and should have been within the `/templates/settings` folder
instead.
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.
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>
We have changed our all instances of list_render to use
simplebar and thus, we will now use simplebar container
to track scroll event for all the lists created by
list_render.
This fixes the bug of new subscribers not rendering on
scrolling at the end of subscriber list in stream settings
and similar bug in some other lists also.
This commit also removes scroll_util.get_list_scrolling_container
function as this is no longer used.
Fixes#15637.
This refactors add_default_stream in zerver/views/streams.py to
take in stream_id as parameter instead of stream_name.
Minor changes have been made to test_subs.py and settings_streams.js
accordingly.
For some widgets we now avoid duplicate redraw
events from this old pattern:
widget = list_render.create(..., {
}).init();
widget.sort(...);
The above code was wasteful and possibly
flicker-y due to the fact that `init` and
`sort` both render.
Now we do this:
widget = list_render.create(..., {
init_sort: [...],
});
For other widgets we just clean up the need
to call `init()` right after `create()`.
We also allow widgets to pass in `sort_fields`
during initialization (since you may want to
have `init_sort` use a custom sort before the
first render.)
Finally, we make the second and third calls
eliminate the prior updates from the previous
widget. This can prevent strange bugs with
double-reversing columns (although that's
been prevented in a better way with a recent
commit), as well as avoiding double work
with sorting.
I pushed this risk commit to the end of
a PR that had a bunch of harmless prep
commits at the front, and I didn't make
it clear enough that the last commit (this
one) hadn't been tested thoroughly.
For the list_render widget, we can simplify
the intialization pretty easily (avoid
extra sorts, for example), but the cache aspects
are still tricky on subsequent calls.
For some widgets we now avoid duplicate redraw
events from this old pattern:
widget = list_render.create(..., {
}).init();
widget.sort(...);
The above code was wasteful and possibly
flicker-y due to the fact that `init` and
`sort` both render.
Now we do this:
widget = list_render.create(..., {
init_sort: [...],
});
For other widgets we just clean up the need
to call `init()` right after `create()`.
We also allow widgets to pass in `sort_fields`
during initialization (since you may want to
have `init_sort` use a custom sort before the
first render.)
When you select a typeahead, it shouldn't
immediately do the action for you; you should
have to hit enter first. Even though 99% of
the time you're gonna confirm the typeahead,
it's jarring when you don't expect it.
You can still add a bunch of default streams
quickly, using only the keyboard, because
we have always had support for the enter
key saving. (and tab and enter also works)
This is a full-stack change:
- server
- JS code
- templates
It's all pretty simple--just use stream_id instead
of stream_name.
I am 99% sure we don't document this API nor use it
in mobile, so it should be a safe change.
We now use the up-to-date info from stream_data
to hydrate the default stream ids. All we need
here in the template is `invite_only` and `name`.
Since we are no longer using data from `page_params`,
we can remove `maybe_update_realm_default_stream_name`.
(If you are wondering if we still get live updates,
we get that via a more upstream call to
update_default_streams_table in the event
dispatching codepath.)
Now if a default stream gets deleted, we just
redraw the table. We always have a small number
of default streams, and the way that we were removing
rows without the actual consent of `list_render` was
really janky (and just a vestige of pre-list-render
code that never got fully ported).
This also makes us consistent with how we handle
added streams (i.e. just call
`update_default_streams_table`).
ASIDE:
Ideally we will update `list_render` at some point to
have an API for adding and removing elements. It does
allow you now to call `data()` to reset its data, but
for now we just build a new `list_render` object every
time.
The filter "callback" was only a "callback" in the
most general sense of the word.
It's just a filter predicate that returns a bool.
This is to prepare for another filtering option,
where the caller can filter the whole list
themselves. I haven't figured out what I will name
the new option yet, but I know I want to make the
two options have specific names.
This commit was originally automatically generated using `tools/lint
--only=eslint --fix`. It was then modified by tabbott to contain only
changes to a set of files that are unlikely to result in significant
merge conflicts with any open pull request, excluding about 20 files.
His plan is to merge the remaining changes with more precise care,
potentially involving merging parts of conflicting pull requests
before running the `eslint --fix` operation.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
With webpack, variables declared in each file are already file-local
(Global variables need to be explicitly exported), so these IIFEs are
no longer needed.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
With perfectScrollbar, we needed to call a function from JavaScript to
enable a scrollbar on a new element, but simplebar has a much simpler
default API one can do by using data-simplebar attributes in the HTML.
So we can delete all the scrollbar creation/deletion code.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
In between releases, the following commit introduced
a bug where we agressively scroll to the top every
place we call `ui.update_scrollbar`:
092b73d0b7
The main symptoms were that the left and right sidebars
would go to the top for things like selecting a topic,
getting activity updates from the server, and resizing
the window. It was very jarring.
The recent commit looked innocuous--the root of the problem
was the original API expressed an intent to scroll to the
top, but didn't actually do it, so it was a bug in hiding.
There are **some** occasions where it's actually appropriate
to scroll to the top, mostly around search filtering, and
in those places we now call the new `ui.reset_scrollbar`
function.
This is a bit of an emergency fix, so particularly with
the settings stuff, we may get more reports of glitches here.
The important thing here is that you almost never want to
reset the scrollTop for sidebars.