Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate left_sidebar and right_sidebar to handlebars. #19048

Merged
merged 2 commits into from Jul 6, 2021

Conversation

aryanshridhar
Copy link
Member

Added a couple of commits to migrate the left_sidebar.html (8ba6d0a) and right_sidebar.html (9c28c91) to handlebars.
The templates (left_sidebar.hbs and right_sidebar.hbs) are then rendered using ui_init.js.

Fixes parts of #18792.

@aryanshridhar
Copy link
Member Author

Updated the pr along with its commit message.
Would be great if @timabbott can have a look at it again :)

@aryanshridhar
Copy link
Member Author

Latest push changes along with rebase -

  • Removed redundant backend tests in test_home due to the template migration.
  • Updated commit message to reflect the same changes.

This commit migrates the `left_sidebar.html` Django template
to handlebars by creating a new file as `left_sidebar.hbs`
which is then rendered using `ui_init` module.

These are the minor changes introduced by virtue of template
migration -
 - The `compute_show_invites_and_add_streams` function now
   only concerns with the invite_to_realm_policy.
 - Renamed the `compute_show_invites_and_add_streams` function
   to `compute_show_invites` due to the above change.
 - Fixes relevant `test_home.py` tests due to the above
   changes.

Fixes part of zulip#18792.
<a href="#recent_topics">
<span class="filter-icon">
<i class="fa fa-clock-o" aria-hidden="true"></i>
</span>
{#- squash whitespace -#}
<span>{{ _('Recent topics') }}</span>
{{!-- squash whitespace --}}
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The squash whitespace comments were not correctly migrated to actually squash whitespace using handlebars whitespace control; I fixed that while merging.

@@ -91,7 +91,7 @@ <h3>{{ _('Loading...') }}</h3>
</div>
<div class="app-main">
<div class="column-left">
{% include "zerver/app/left_sidebar.html" %}
<div id = "left-sidebar-holder"></div>
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided it was better to add left-sidebar-container as an ID on the column-left class here, to avoid creating an additional unnecessary div.

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note that we don't use space around the = in our HTML style.

// to build the page layout early so that it's rendered content
// can be used by other modules.
// For example- Populating the stream lists/populating private messages.
initialize_left_sidebar();
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rewrote the comments, since this should be next to compose and share its comment.

This commit migrates the `right_sidebar.html` Django template
to handlebars by creating a new file as `right_sidebar.hbs`
which is then rendered using `ui_init` module.

It also removes the tests in `test_home` due to the template
migration, since these elements aren't rendered on the backend
anymore.

We also remove `test_compute_show_invites_and_add_streams*`.

Fixes part of zulip#18792.
@@ -138,6 +139,14 @@ function initialize_left_sidebar() {
$("#left-sidebar-holder").html(rendered_sidebar);
}

function initialize_right_sidebar() {
const rendered_sidebar = render_right_sidebar({
can_invite_others_to_realm: page_params.show_invites,
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this to settings_data.user_can_invite_others_to_realm(), since we can compute this on the frontend.

This may mean we get to remove compute_show_invites as well in a follow-up PR.

@timabbott timabbott merged commit 1339983 into zulip:master Jul 6, 2021
@timabbott
Copy link
Sponsor Member

Merged after making that batch of changes, thanks @aryanshridhar!

Can you look at whether we get to delete the backend can_show_invites function as well? I think the answer is yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants