refactor: Simplify logic for circles.

We now have a function get_user_circle_class that returns one of these values: "user_circle_green" "user_circle_orange" "user_circle_empty" And we put that in the templates. And then CSS renders the circle of the appropriate color. The unit tests now explicitly capture whether we are rendering the correct kind of circle.
2025-11-04 05:53:43 +00:00 · 2019-02-17 01:10:42 +00:00
parent ba91f628c7
commit 1adcaad04a
11 changed files with 58 additions and 35 deletions
--- a/frontend_tests/node_tests/activity.js
+++ b/frontend_tests/node_tests/activity.js
@@ -607,7 +607,7 @@ run_test('insert_one_user_into_empty_list', () => {
    clear_buddy_list();
    activity.redraw_user(alice.user_id);
    assert(appended_html.indexOf('data-user-id="1"') > 0);
-    assert(appended_html.indexOf('user_active') > 0);
+    assert(appended_html.indexOf('user_circle_green') > 0);
 });

 reset_setup();
@@ -622,11 +622,11 @@ run_test('insert_alice_then_fred', () => {

    activity.redraw_user(alice.user_id);
    assert(appended_html.indexOf('data-user-id="1"') > 0);
-    assert(appended_html.indexOf('user_active') > 0);
+    assert(appended_html.indexOf('user_circle_green') > 0);

    activity.redraw_user(fred.user_id);
    assert(appended_html.indexOf('data-user-id="2"') > 0);
-    assert(appended_html.indexOf('user_active') > 0);
+    assert(appended_html.indexOf('user_circle_green') > 0);
 });

 reset_setup();
@@ -641,7 +641,7 @@ run_test('insert_fred_then_alice_then_rename', () => {

    activity.redraw_user(fred.user_id);
    assert(appended_html.indexOf('data-user-id="2"') > 0);
-    assert(appended_html.indexOf('user_active') > 0);
+    assert(appended_html.indexOf('user_circle_green') > 0);

    var fred_stub = $.create('fred-first');
    buddy_list_add(fred.user_id, fred_stub);
@@ -653,7 +653,7 @@ run_test('insert_fred_then_alice_then_rename', () => {

    activity.redraw_user(alice.user_id);
    assert(inserted_html.indexOf('data-user-id="1"') > 0);
-    assert(inserted_html.indexOf('user_active') > 0);
+    assert(inserted_html.indexOf('user_circle_green') > 0);

    // Next rename fred to Aaron.
    const fred_with_new_name = {
--- a/frontend_tests/node_tests/buddy_data.js
+++ b/frontend_tests/node_tests/buddy_data.js
@@ -77,6 +77,20 @@ function activate_people() {
 make_people();
 activate_people();

+run_test('user_circle', () => {
+    assert.equal(buddy_data.get_user_circle_class(selma.user_id), 'user_circle_green');
+    user_status.set_away(selma.user_id);
+    assert.equal(buddy_data.get_user_circle_class(selma.user_id), 'user_circle_empty');
+    user_status.revoke_away(selma.user_id);
+    assert.equal(buddy_data.get_user_circle_class(selma.user_id), 'user_circle_green');
+
+    assert.equal(buddy_data.get_user_circle_class(me.user_id), 'user_circle_green');
+    user_status.set_away(me.user_id);
+    assert.equal(buddy_data.get_user_circle_class(me.user_id), 'user_circle_empty');
+    user_status.revoke_away(me.user_id);
+    assert.equal(buddy_data.get_user_circle_class(me.user_id), 'user_circle_green');
+});
+
 run_test('buddy_status', () => {
    assert.equal(buddy_data.buddy_status(selma.user_id), 'active');
    user_status.set_away(selma.user_id);
--- a/frontend_tests/node_tests/popovers.js
+++ b/frontend_tests/node_tests/popovers.js
@@ -157,7 +157,7 @@ run_test('sender_hover', () => {
                user_id: 42,
                user_time: undefined,
                user_type: i18n.t('Member'),
-                type: 'offline',
+                user_circle_class: 'user_circle_empty',
                user_last_seen_time_status: 'translated: More than 2 weeks ago',
                pm_with_uri: '#narrow/pm-with/42-alice',
                sent_by_uri: '#narrow/sender/42-alice',
--- a/frontend_tests/node_tests/templates.js
+++ b/frontend_tests/node_tests/templates.js
@@ -1422,19 +1422,16 @@ run_test('user_group_info_popover_content', () => {
        group_description: 'groupDescription',
        members: [
            {
-                presence_status: 'active',
                full_name: 'Active Alice',
                user_last_seen_time_status: 'time',
                is_bot: false,
            },
            {
-                presence_status: 'offline',
                full_name: 'Bot Bob',
                user_last_seen_time_status: 'time',
                is_bot: true,
            },
            {
-                presence_status: 'offline',
                full_name: 'Inactive Imogen',
                user_last_seen_time_status: 'time',
                is_bot: false,
@@ -1445,8 +1442,6 @@ run_test('user_group_info_popover_content', () => {
    var html = render('user_group_info_popover_content', args);

    var allUsers = $(html).find("li");
-    assert.equal(allUsers[0].classList.contains("user_active"), true);
-    assert.equal(allUsers[2].classList.contains("user_offline"), true);
    assert.equal($(allUsers[0]).text().trim(), 'Active Alice');
    assert.equal($(allUsers[1]).text().trim(), 'Bot Bob');
    assert.equal($(allUsers[2]).text().trim(), 'Inactive Imogen');
--- a/static/js/buddy_data.js
+++ b/static/js/buddy_data.js
@@ -32,6 +32,22 @@ var fade_config = {
    },
 };

+exports.get_user_circle_class = function (user_id) {
+    var status = exports.buddy_status(user_id);
+
+    switch (status) {
+    case 'active':
+        return 'user_circle_green';
+    case 'idle':
+        return 'user_circle_orange';
+    case 'away_them':
+    case 'away_me':
+        return 'user_circle_empty';
+    default:
+        return 'user_circle_empty';
+    }
+};
+
 exports.level = function (user_id) {
    if (people.is_my_user_id(user_id)) {
        // Always put current user at the top.
@@ -157,7 +173,7 @@ exports.user_title = function (user_id) {
 };

 exports.info_for = function (user_id) {
-    var buddy_status = exports.buddy_status(user_id);
+    var user_circle_class = exports.get_user_circle_class(user_id);
    var person = people.get_person_from_user_id(user_id);
    var my_user_status = exports.my_user_status(user_id);
    var title = exports.user_title(user_id);
@@ -169,7 +185,7 @@ exports.info_for = function (user_id) {
        my_user_status: my_user_status,
        is_current_user: people.is_my_user_id(user_id),
        num_unread: get_num_unread(user_id),
-        type: buddy_status,
+        user_circle_class: user_circle_class,
        title: title,
    };
 };
--- a/static/js/popovers.js
+++ b/static/js/popovers.js
@@ -137,7 +137,7 @@ function render_user_info_popover(user, popover_element, is_sender_popover, priv
        is_me: is_me,
        is_sender_popover: is_sender_popover,
        pm_with_uri: hash_util.pm_with_uri(user.email),
-        type: buddy_data.buddy_status(user.user_id),
+        user_circle_class: buddy_data.get_user_circle_class(user.user_id),
        private_message_class: private_msg_class,
        sent_by_uri: hash_util.by_sender_uri(user.email),
        show_user_profile: !(user.is_bot || page_params.custom_profile_fields.length === 0),
@@ -331,7 +331,7 @@ function fetch_group_members(member_ids) {
        })
        .map(function (p) {
            return Object.assign({}, p, {
-                presence_status: buddy_data.buddy_status(p.user_id),
+                user_circle_class: buddy_data.get_user_circle_class(p.user_id),
                is_active: people.is_active_user_for_popover(p.user_id),
                user_last_seen_time_status: user_last_seen_time_status(p.user_id),
            });
--- a/static/styles/right-sidebar.scss
+++ b/static/styles/right-sidebar.scss
@@ -59,16 +59,16 @@
    display: inline-block;
 }

-.user-status-indicator,
-.user_circle_fraction {
-    display: block;
+.user_circle,
+#group-pms .user_circle_fraction {
    width: 8px;
    height: 8px;
    margin: 0px 5px;
 }

-.user_unknown .user-status-indicator {
-    display: none;
+#user_presences .user_circle,
+#group-pms .user_circle_fraction {
+    display: block;
 }

 #user_presences a,
--- a/static/styles/user_circles.scss
+++ b/static/styles/user_circles.scss
@@ -1,4 +1,6 @@
-.user-status-indicator,
+.user_circle_green,
+.user_circle_orange,
+.user_circle_empty,
 .user_circle_fraction {
    border-radius: 50%;
    border: 1px solid;
@@ -7,16 +9,16 @@
    top: 5px;
 }

-.user_active .user-status-indicator {
+.user_circle_green {
    background-color: hsl(106, 74%, 44%);
 }

-.user_active .user-status-indicator,
+.user_circle_green,
 .user_circle_fraction {
    border-color: hsl(106, 74%, 44%);
 }

-.user_idle .user-status-indicator {
+.user_circle_orange {
    border-color: hsl(29, 84%, 51%);
    background-color: hsl(29, 84%, 51%);

@@ -29,11 +31,7 @@
    filter: progid:DXImageTransform.Microsoft.gradient( startColorstr='#ffffff', endColorstr='#ec7e18', GradientType=0 ); /* IE6-9; filters only work with hex colors */
 }

-// We show "away" users identically to long-gone
-// users, but this may change.
-.user_away_me .user-status-indicator,
-.user_away_them .user-status-indicator,
-.user-status-indicator {
+.user_circle_empty {
    background-color: none;
    border-color: hsl(0, 0%, 50%);
 }
--- a/static/templates/user_group_info_popover_content.handlebars
+++ b/static/templates/user_group_info_popover_content.handlebars
@@ -8,12 +8,12 @@
 <hr>
 <ul class="nav nav-list member-list">
    {{#each members}}
-    <li class="user_{{presence_status}}">
+    <li>
        {{#if is_active }}
            {{#if is_bot}}
            <i class="zulip-icon bot" aria-hidden="true"></i>
            {{else}}
-            <span class="user-status-indicator popover_user_presence" title="{{user_last_seen_time_status}}"></span>
+            <span class="user_circle {{user_circle_class}}  popover_user_presence" title="{{user_last_seen_time_status}}"></span>
            {{/if}}
        {{/if}}
        <span>{{full_name}}</span>
--- a/static/templates/user_info_popover_content.handlebars
+++ b/static/templates/user_info_popover_content.handlebars
@@ -1,13 +1,13 @@
 {{! Contents of the "message info" popup }}
 <ul class="nav nav-list actions_popover info_popover_actions" data-user-id="{{user_id}}">
    <div class="popover_info">
-        <li class="user_{{type}}">
+        <li>
            <b>{{user_full_name}}</b>
            {{#if is_active }}
                {{#if is_bot}}
                <i class="zulip-icon bot" aria-hidden="true"></i>
                {{else}}
-                <span class="user-status-indicator popover_user_presence" title="{{user_last_seen_time_status}}"></span>
+                <span class="{{user_circle_class}} user_circle popover_user_presence" title="{{user_last_seen_time_status}}"></span>
                {{/if}}
            {{/if}}
        </li>
--- a/static/templates/user_presence_row.handlebars
+++ b/static/templates/user_presence_row.handlebars
@@ -1,6 +1,6 @@
-<li data-user-id="{{user_id}}" class="user_sidebar_entry {{#if num_unread}} user-with-count {{/if}} narrow-filter user_{{type}} {{#if faded}} user-fade {{/if}}">
+<li data-user-id="{{user_id}}" class="user_sidebar_entry {{#if num_unread}} user-with-count {{/if}} narrow-filter {{#if faded}} user-fade {{/if}}">
    <div class="selectable_sidebar_block">
-        <span class="user-status-indicator"></span>
+        <span class="{{user_circle_class}} user_circle"></span>
        <a class="user-presence-link"
          href="{{href}}"
          data-user-id="{{user_id}}"