Going through the description of commit
a150b9b0ae is highly recommended since
this is a related issue.
We had received a complaint on chat.zulip.org about navigation with
the keyboard `n` key being significantly slow (~5 seconds), the first
time `n` was pressed when starting from the recent topics view.
It was difficult to reproduce the amount of lag that was reported, but
running chrome with the profile tab set to 4x slowdown helped get
close to it.
Based on profiling from the original report, as well as locally with
chrome set to 4x slowdown, led to the realisation that recent topics
to stream navigation involved a lot of dom thrashing, and so this
series of commits aims to prevent this path from causing forced
reflows.
In this commit, we reorder the calls to $(...).show() in
recent_topics_ui.hide(), this prevents the first reflow in this path,
most likely because displaying message_feed_container before
message_view_header_underpadding was guaranteed to cause style
recalculations since the underpadding is visually above the message
container.
This causes a net 60 ms decrease in the first renarrow, an ~ 70 ms
increase in the second renarrow and an ~ 5 ms increase in the third
renarrow but, more importantly, it eliminates one reflow and sets on a
path where we can achieve strong gains in subsequent commits.
Since the position of topic in recent topics can change, we
focus the last selected topic using the `topic_key` instead
of relying `row_focus` value which is incorrect.
When user is scrolling, we simply keep the center element in
focus.
When user is using hotkeys, we keep the focused element in
center.
When user is using keyboard, we need to always keep the
"focused" topic in visible scrolling area.
We determine if the topic row is above or below the visible
area and scroll half_height_of_visible_area so that the selected
topic is visible.
This gives a nice navigation experience for both the views.
Reduced height of recent topics table to account for
compose box so that focused element is not below compose box.
Previously, navigating from any stream to the recent topics view would
cause a forced reflow every time we checked `is_visible()` because it
would call `$("#recent_topics_view").is(":visible")`.
The reason for this is related to how browsers ship frames, the
process follows these steps:
JavaScript > style calculations > layout > paint > composite.
(The layout step is called Reflow in firefox.)
Typically, the browser will handle these steps in the most optimal
manner possible, delaying expensive operations until they're needed.
However, it is possible to cause the browser to perform a layout
earlier than necessary. An example of this is what we previously did:
When we call `top_left_corner.narrow_to_recent_topics()`, we ask to
add a class via `.addClass()`, this schedules a Style Recalculation,
then, when we call `message_view_header.make_message_view_header()` it
calls `recent_topics_util.is_visible()` which calls
`$("#recent_topics_view").is(":visible")`.
Before the browser can get this value, it realizes that our dom was
invalidated by `.addClass()` and so it must execute the scheduled
Style Recalculation and cause a layout.
This is called a forced synchronous layout.
This commit adds a JavaScript variable representing the visible state,
in order to prevent the above behavior.
This commit reduces the main thread run time of
`build_message_view_header` from 131.81 ms to 5.20 ms.
Unfortunately we still have the case where
`recent_topics_ui.revive_current_focus()` calls
`recent_topics_ui.set_table_focus()` which causes a reflow.
However, by eliminating this reflow we still save ~100ms.
(It's important to note that we only save this sometimes, as other
things can still cost us a reflow.)
Further reading: https://developers.google.com/web/fundamentals/
performance/rendering/avoid-large-complex-layouts-and-layout-thrashing
We can use this format to display text in plural form or not based
on a number. This helps translators easily translate text and
users get a better formatted text.
Since, the filter button is replaced with a different button
after click, the `current_focus_elem` points at incorrect
element. `revive_current_focus` follows a good
method to locate the filter button, hence
we use it to correct the element `current_focus_elem` points at.
d779a1c tweaked `get_full_datetime` to return a string instead
of a {date, time} object. This function is used for recent topics
too but wasn't fixed to use the string.
This resulted in showing 'undefined undefined' in tooltip.
We split recent_topics module into recent_topics_(ui + data + util).
This allows us to reduce cyclical dependencies which were
created due to large list of imports in recent topics. Also, this
refactor on its own makes sense.
Aman Agrawal
YashRE42
Dinesh
Priyank Patel