Previously, we had to fiddle with the generated HTML to update
individual values. Now, we can simply ask the widget to rerender
the row that we updated.
This is done by passing an html_selector function that returns
a selector for the rendered item.
If:
- we do not provide html_selector function
- item is not currently rendered
- new html is not a string.
then the render_item() call is a noop.
We do not shift much of the validation logic here just
yet. This function has been declared at the top of the
file to act as usage docs for the widget as well, in
terms of what combinations of opts are valid and what
are not.
We remove the "GROUP PMs" section that used
to be in the lower right sidebar.
Most of this is straightforward code removal.
A couple quick notes:
- The message fetching code now just
calls `huddle_data.process_loaded_messages`,
which we still need for search suggestions.
We removed `activity.process_loaded_messages`.
- The `huddle_data.process_loaded_messages`
function no longer needs to return `need_resize`.
- In `resize.js` we now just calculate
`res.buddy_list_wrapper_max_height` directly
from `usable_height`.
This fixes the calculation for how far from the
top of the viewport we think #right_sidebar's
top is. To fully explain this commit requires
some background info.
Normally `#right-sidebar` has 50px of top margin
and 0px of top padding. And our `resize.js`
calculations have been accurate for the normal
case.
But when you are in the so-called `.expanded` mode
(i.e. when you're in a narrow window) we split up the
50px as follows:
- 40px margin
- 10px padding
Why don't I make the CSS just be more consistent here?
- If you go to 50px in the "expanded" mode
you mostly cover up the right scrollbar,
except for the 10px gutter that is below
the 40px-tall `.header` section. To fully
cover it we apparently want the padding;
otherwise you see a small, unusable remnant
of the scrollbar which just looks funny.
- If we were to make the "regular" right sidebar
just always have the 40/10 split, then we
would start to diverge from the left sidebar,
which is currently 50/0 as well.
- If we went to make both the left and the right
sidebars 40/10 split, well, that's just an
even riskier change.
So instead I fix the resize calculation:
I just calculate the actual `top` position.
Is any of this actually user-facing?
Yes. Now if a user is a narrow window and
they open the buddy list, we will make
the buddy list 10px smaller to account for
the padding. This makes it less likely for
the invite link to get squeezed out.
We'll use this in the next commit.
Note that there's a minor change in the order
in which we apply new heights--we now
do sidebars before bottom whitespace.
We had a bunch of places where we
were calling `resize.resize_bottom_whitespace`
with no arguments, which has been a no-op
since the below commit that removed support
for our `autoscroll_forever` option:
fa44d2ea69
With the `autoscroll_forever` options things
like opening/closing the compose box could
alter how much bottom whitespace you'd want,
but we stopped supporting that feature in
2017.
Since then bottom_whitespace has just always
been 40% of the viewport size. So we only need
to change it on actual resize events.
It's worth noting that we still call
`resize_bottom_whitespace` indirectly in many
places, via `resize_page_components`, and
the latter actually causes
`resize_bottom_whitespace` to do real work,
but that work is redundant for most of those
codepaths, since they're not triggered by
changes to the viewport. So there are other
opportunities for cleanup.
The `buddy_list_wrapper` has zeros margins, so it's
just noise in the current calculations. You can
verify this pretty easily with console statements,
as well as looking at the code. I tried it with
various permutations of narrow windows and display
settings.
The header is 40px tall, with a 10px gutter
below it, which means the top of our sidebars
are 50px from the top of the viewport.
Now all the places that share these values
use `$header_right` and related values.
This is pretty easy to test out by just doubling
or tripling the two numbers at the top of the
file.
The section for `@media (max-width: 500px)` seems
to have its own smaller values for things like
the `height` of `.header`, so I left it alone.
Trigger realm icon upload by clicking on realm icon element itself
rather than having a big upload button and to match our user avatar UI.
Added new spinner over the icon element itself to show while
uploading realm icon.
The bug this was working around does not affect our current toolchain,
as confirmed by grepping through the minified output.
(Also, this linter rule only matched calc(x + y) with two arguments
and we were already using calc($far_left_gutter_size + $left_col_size
+ 4px).)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit has us piggy back the conditional for narrow_description
off of the conditional for sub_count, the reason for this approach is
that "narrow_description" needs to handle four unique cases:
- The stream exists and has a description.
- The stream exists and does not have a description.
- The stream does not exist and we must render appropriate text.
- We are not in a stream narrow and the span should not be rendered.
By piggy backing off of sub count we can get the first and last case,
with the inner conditional (on rendered_narrow_description) handling
the second case and the tab_bar.js passing appropriate values to the
template to handle the third case.
This unfortunately makes the template more brittle such that breakage
of the subcount can cause breakage (non rendering) of the description
as well.
Computed indexes into these raw objects should be guarded with
Object.prototype.hasOwnProperty; make our accessors do this
automatically and use them consistently.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The previous styling was brittle and ended up breaking in very small
phone-size views with the text overflowing the boundaries of the page.
The right fix is to move those heading outside the portico-header
class, since the CSS for that isn't generally appropriate here.
This is a prep commit which combines the previous `#searchbox`
block with the newly updated `#searchbox_legacy` block which
contains the modifications related to the new navbar display.
This only consists of changes to `#searchbox` and is still broken.
But it integrates the searchbox with the new tab_bar changes so that
only one searchbox is shown (instead of two, previously).
This is helpful because if the user pastes multiple queries in the
searchbox and there are invalid search operators, then it is visible
through the typeahead.
The main reasoning for this change is as follows:
* When the search bar contains multiple search queries
but no search results, the last search operand does
not get displayed.
This happens due to the fact that filter object
contained 2 terms having the operator key value as
"search" instead of a single term where operator is
"search" and operand is a single string containing
the space seperated search queries. This condition
occurs for search_pills_enabled case only because
we used to Filter.parse the query twice
(once for the `base_operators` and once for the
`suggestion_operator instead of doing both at once).
Thus the `search_query` value inside the
`narrow.show_search_query` function which only
selected the operands of the first term displayed
an incomplete result.
* Another benefit of this commit is to display the narrow
operators in the URL fragment the same way as when
search_pills_enabled = False.
For example, On entering the queries in the mentioned
order -> 'is: starred', 'abc', 'def', 'is: private',
'ghi'. This is the URL:
Previously:
/#narrow/is/starred/is/private/search/abc.20def/search/ghi
Now (same as pills disabled case):
/#narrow/is/starred/is/private/search/abc.20def.20ghi
* We are also able to de-duplicate the non-typeahead search
query code path.
As mentioned in the comment for `KEY.BACKSPACE` event
in `input_pills.js`, we do normal character deletion
if there is input present. However this wasn't the case
if spaces were present. Also the input wasn't cleared
after the last pill was removed.
Thus `trim()` is removed from the input length check and
the new pill is still created from the trimmed value.
We can remove the typeahead by clicking outside the search box
after we have entered the search string to be filtered and then
focus on the searchbox and press enter or just by pressing enter
on an empty string.
Previously, the narrow would just deactivate for the above condition
as the searchbox value which was passed as the raw_operators parameter
to the narrow.activate function was empty.
This happened because we called the activate function on pressing
enter for the keyup event, while the keydown event in the parent
container made a pill from the text and cleared the input. (as
mentioned in the comment for `KEY.ENTER` case in `input_pill.js`)
The people.js tests were using _add_user function to add
cross realm bots. The problem is that _add_user function
doesn't properly simulates the adding process as it doesn't
add the user in cross_realm_dict as well.
To solve this and eliminate the need of calling
people.initialize(), which means the params obj needs to be
defined, we extracted the whole logic of adding a cross realm
user into a separete function, add_cross_realm_user.
This fixes some issues with unclear terminology and visual styling in
the pages for the new free trial.
There's probably more we can and should usefully do in the future.
This makes it so that search_suggestion.js
does not depend on activity.js.
That dependency hasn't really been "elegant"
for quite some time, but it will become particularly
unnecessary when we go to remove the "Group PMs"
section from the right sidebar.
This commit introduces a temporary wart
where we have these two functions with the
same name in a sort of unnecessarily
complicated code stack:
activity.process_loaded_messages
huddle_data.process_loaded_messages
But we will eliminate the former function
very soon, and our message-related codepaths
will just call the `huddle_data` version
directly.
TESTING NOTES:
Now that `huddle_data` is a tiny leaf
module, it's super easy to just use the
real implementation of what was formerly
called `activity.get_huddles()` (and is
now in `huddle_data`).
When I first wrote this commit, introducing
the real implementation of `get_huddles` exposed
some bugs that I fixed in the immediately
prior commits to this.
When the tests were originally written,
I believe `activity.js` had some annoying
`jQuery` dependencies that made it hard
to unit test against. We've slimmed it over
time to be mostly just a "controller" module.
But even in its current state it would have
been a bit of a bloated dependency.
The other friction for using the actual
version of `get_huddles` was setting up
the message data, but that's pretty minor.
If you have a group PM where some users have
three-digit user_ids and some with four-digit
user_ids (or similar), a huddle could effectively
be ignored when determining the order of
search search suggestions.
Basically, we need a way to canonically sort
user_ids in "huddle" strings, and it's somewhat
arbitrary whether you sort lexically or sort
numerically, but you do need to be consistent
about it.
And JS is not exactly helpful here:
> [99, 101].sort()
[ 101, 99 ]
This is a pretty obscure bug with pretty low
user-facing consequences, and it was never
reported to us as far as I know, but the fix
here is pretty straightforward.
We have had similar bugs of slightly more consequence
in the past. The reason this bug has shown
up multiple times in our codebase is that every
component that deals with huddles has slightly
different forces that determine how it wants
to serialize the huddle. It's just one of those
annoying things. Plus, bugs with group PMs
do tend to escape detection, since most people
spend most of their time either on streams
or in 1:1 PMs.
This is a pure code extraction. The current
code is buggy with respect to user_ids with
different lengths of digits, i.e. it does
a naive lexical sort instead of a numerical
sort. We'll fix that in the next commit.
We already have a loading indicator for fetching older
messages. Thus it makes sense to implement the same
for displaying newer messages.
We set the display of `bottom-messages-logo` to none,
to prevent displaying two loading indicators during
the initial message load.
Fixes#15060.
`loading_more_messages_indicator` is renamed to
`loading_older_messages_indicator`.
This is a prep commit to introduce
`loading_newer_messages_indicator`.
This commit fixes the alignment of emoji in the navbar by removing a
redundant style which was breaking the emoji alignment.
This block is probably just a remanent from WIP development of this
version of the navbar & its inclusion on master was as an oversight.
This commit replaces fa-file-text-o with fa-file-code-o which is a
better signal for the "view source" action. It also deletes a single
line comment that had suggested the change once we moved into font
awesome 4, which Aditya Bansal <adi.bansal241996@gmail.com> helped
out in doing, first via
91962aa6ab and most recently via
75ae94e459 with several commits in
between.
This commit adds code to live update the message edit history.
Message edit history is fetched and rendered again if the edit
history modal is open.
This also adds 'data-message-id' attribute to 'message-history'
when opening history modal element which is used for checking
whether the history modal opened is of the message which is
edited.
Fixes#15051.
Siddharth Varshney
Rohitt Vashishtha
Mateusz Mandera
Steve Howell
jagansivam28
Anders Kaseorg
Bhanu Gupta
Tim Abbott
YashRE42
Ryan Rehman
clarammdantas
Vishnu KS
sahil839