UX: Read indicator improvements. (#8049)

* The read indicator now shows up when no member has read the last post of the topic (written by a non-member) * The read indicator works on mobile and receives live updates from message bus * The icon we display in the topic list was changed * Added a title to the indicator to indicate its purpose when hovering over it
2024-11-22 08:57:10 -06:00 · 2019-08-29 12:03:43 -03:00 · 2019-08-29 12:03:43 -03:00 · ebb389ef8a
commit ebb389ef8a
parent 1e89939383
12 changed files with 107 additions and 37 deletions
--- a/app/assets/javascripts/discourse/components/basic-topic-list.js.es6
+++ b/app/assets/javascripts/discourse/components/basic-topic-list.js.es6
@ -33,6 +33,51 @@ export default Ember.Component.extend({
    }
  },

+  didInsertElement() {
+    this._super(...arguments);
+
+    this.topics.forEach(topic => {
+      const includeUnreadIndicator =
+        typeof topic.unread_by_group_member !== "undefined";
+
+      if (includeUnreadIndicator) {
+        const unreadIndicatorChannel = `/private-messages/unread-indicator/${topic.id}`;
+        this.messageBus.subscribe(unreadIndicatorChannel, data => {
+          const nodeClassList = document.querySelector(
+            `.indicator-topic-${data.topic_id}`
+          ).classList;
+
+          if (data.show_indicator) {
+            nodeClassList.remove("read");
+          } else {
+            nodeClassList.add("read");
+          }
+        });
+      }
+    });
+  },
+
+  willDestroyElement() {
+    this._super(...arguments);
+
+    this.topics.forEach(topic => {
+      const includeUnreadIndicator =
+        typeof topic.unread_by_group_member !== "undefined";
+
+      if (includeUnreadIndicator) {
+        const unreadIndicatorChannel = `/private-messages/unread-indicator/${topic.id}`;
+        this.messageBus.unsubscribe(unreadIndicatorChannel);
+      }
+    });
+  },
+
+  @computed("topics")
+  showUnreadIndicator(topics) {
+    return topics.some(
+      topic => typeof topic.unread_by_group_member !== "undefined"
+    );
+  },
+
  click(e) {
    // Mobile basic-topic-list doesn't use the `topic-list-item` view so
    // the event for the topic entrance is never wired up.
--- a/app/assets/javascripts/discourse/components/topic-list-item.js.es6
+++ b/app/assets/javascripts/discourse/components/topic-list-item.js.es6
@ -38,16 +38,16 @@ export const ListItemDefaults = {
  didInsertElement() {
    this._super(...arguments);

-    if (this.includeReadIndicator) {
-      this.messageBus.subscribe(this.readIndicatorChannel, data => {
+    if (this.includeUnreadIndicator) {
+      this.messageBus.subscribe(this.unreadIndicatorChannel, data => {
        const nodeClassList = document.querySelector(
          `.indicator-topic-${data.topic_id}`
        ).classList;

        if (data.show_indicator) {
-          nodeClassList.remove("unread");
+          nodeClassList.remove("read");
        } else {
-          nodeClassList.add("unread");
+          nodeClassList.add("read");
        }
      });
    }
@ -56,24 +56,24 @@ export const ListItemDefaults = {
  willDestroyElement() {
    this._super(...arguments);

-    if (this.includeReadIndicator) {
-      this.messageBus.unsubscribe(this.readIndicatorChannel);
+    if (this.includeUnreadIndicator) {
+      this.messageBus.unsubscribe(this.unreadIndicatorChannel);
    }
  },

  @computed("topic.id")
-  readIndicatorChannel(topicId) {
-    return `/private-messages/read-indicator/${topicId}`;
+  unreadIndicatorChannel(topicId) {
+    return `/private-messages/unread-indicator/${topicId}`;
  },

-  @computed("topic.read_by_group_member")
-  unreadClass(readByGroupMember) {
-    return readByGroupMember ? "" : "unread";
+  @computed("topic.unread_by_group_member")
+  unreadClass(unreadByGroupMember) {
+    return unreadByGroupMember ? "" : "read";
  },

-  @computed("topic.read_by_group_member")
-  includeReadIndicator(readByGroupMember) {
-    return typeof readByGroupMember !== "undefined";
+  @computed("topic.unread_by_group_member")
+  includeUnreadIndicator(unreadByGroupMember) {
+    return typeof unreadByGroupMember !== "undefined";
  },

  @computed
--- a/app/assets/javascripts/discourse/templates/list/topic-list-item.raw.hbs
+++ b/app/assets/javascripts/discourse/templates/list/topic-list-item.raw.hbs
@ -20,14 +20,10 @@
    {{~topic-featured-link topic}}
    {{~/if}}
    {{~raw-plugin-outlet name="topic-list-after-title"}}
+    {{~raw "list/unread-indicator" includeUnreadIndicator=topic.unread_by_group_member topicId=topic.id ~}}
    {{~#if showTopicPostBadges}}
    {{~raw "topic-post-badges" unread=topic.unread newPosts=topic.displayNewPosts unseen=topic.unseen url=topic.lastUnreadUrl newDotText=newDotText}}
    {{~/if}}
-    {{~#if includeReadIndicator}}
-      <span class='read-indicator indicator-topic-{{topic.id}} {{unreadClass}}'>
-        {{~d-icon "book-reader"}}
-      </span>
-    {{~/if}}
  </span>
  <div class="link-bottom-line">
    {{#unless hideCategory}}
--- a/app/assets/javascripts/discourse/templates/list/unread-indicator.raw.hbs
+++ b/app/assets/javascripts/discourse/templates/list/unread-indicator.raw.hbs
@ -0,0 +1,5 @@
+{{~#if includeUnreadIndicator~}}
+  &nbsp;<span class='badge badge-notification unread-indicator indicator-topic-{{topicId}} {{unreadClass}}' title='{{i18n "topic.unread_indicator"}}'>
+    {{~d-icon "asterisk"}}
+  </span>
+{{~/if}}
--- a/app/assets/javascripts/discourse/templates/mobile/components/basic-topic-list.hbs
+++ b/app/assets/javascripts/discourse/templates/mobile/components/basic-topic-list.hbs
@ -16,6 +16,9 @@
            <div class='main-link'>
              {{topic-status topic=t}}
              {{topic-link t}}
+              {{raw "list/unread-indicator" includeUnreadIndicator=showUnreadIndicator
+                                            topicId=t.id 
+                                            unreadClass=(if t.unread_by_group_member "" "read")}}
              {{#if t.unseen}}
                <span class="badge-notification new-topic"></span>
              {{/if}}
--- a/app/assets/stylesheets/common/base/_topic-list.scss
+++ b/app/assets/stylesheets/common/base/_topic-list.scss
@ -133,11 +133,15 @@
    .raw-topic-link > * {
      pointer-events: none;
    }
+  }

-    .read-indicator {
-      &.unread {
-        display: none;
-      }
+  .unread-indicator {
+    &.read {
+      display: none;
+    }
+    .d-icon {
+      vertical-align: 0em;
+      font-size: $font-down-5;
    }
  }

--- a/app/models/topic_tracking_state.rb
+++ b/app/models/topic_tracking_state.rb
@ -347,7 +347,7 @@ SQL

    if topic.private_message?
      groups = read_allowed_groups_of(topic)
-      update_topic_list_read_indicator(topic, groups, topic.highest_post_number, user_id, false)
+      update_topic_list_read_indicator(topic, groups, topic.highest_post_number, user_id, true)
    end
  end

@ -358,7 +358,7 @@ SQL
      groups = read_allowed_groups_of(topic)
      post = Post.find_by(topic_id: topic.id, post_number: last_read_post_number)
      trigger_post_read_count_update(post, groups)
-      update_topic_list_read_indicator(topic, groups, last_read_post_number, user_id, true)
+      update_topic_list_read_indicator(topic, groups, last_read_post_number, user_id, false)
    end
  end

@ -370,23 +370,23 @@ SQL
      .group('groups.id')
  end

-  def self.update_topic_list_read_indicator(topic, groups, last_read_post_number, user_id, read_event)
+  def self.update_topic_list_read_indicator(topic, groups, last_read_post_number, user_id, write_event)
    return unless last_read_post_number == topic.highest_post_number
-    message = { topic_id: topic.id, show_indicator: read_event }.as_json
+    message = { topic_id: topic.id, show_indicator: write_event }.as_json
    groups_to_update = []

    groups.each do |group|
      member = group.members.include?(user_id)

-      member_writing = (!read_event && member)
-      non_member_reading = (read_event && !member)
+      member_writing = (write_event && member)
+      non_member_reading = (!write_event && !member)
      next if non_member_reading || member_writing

      groups_to_update << group
    end

    return if groups_to_update.empty?
-    MessageBus.publish("/private-messages/read-indicator/#{topic.id}", message, user_ids: groups_to_update.flat_map(&:members))
+    MessageBus.publish("/private-messages/unread-indicator/#{topic.id}", message, user_ids: groups_to_update.flat_map(&:members))
  end

  def self.trigger_post_read_count_update(post, groups)
--- a/app/serializers/listable_topic_serializer.rb
+++ b/app/serializers/listable_topic_serializer.rb
@ -26,7 +26,7 @@ class ListableTopicSerializer < BasicTopicSerializer
             :bookmarked,
             :liked,
             :unicode_title,
-             :read_by_group_member
+             :unread_by_group_member

  has_one :last_poster, serializer: BasicUserSerializer, embed: :objects

@ -122,15 +122,15 @@ class ListableTopicSerializer < BasicTopicSerializer
    PinnedCheck.unpinned?(object, object.user_data)
  end

-  def read_by_group_member
+  def unread_by_group_member
    # object#last_read_post_number is an attribute selected from a joined table.
    # See TopicQuery#append_read_state for more information.
    return false unless object.respond_to?(:last_read_post_number)

-    object.last_read_post_number >= object.highest_post_number
+    object.last_read_post_number < object.highest_post_number
  end

-  def include_read_by_group_member?
+  def include_unread_by_group_member?
    !!object.topic_list&.publish_read_state
  end

--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@ -1986,6 +1986,7 @@ en:
      group_request: "You need to request membership to the `{{name}}` group to see this topic"
      group_join: "You need join the `{{name}}` group to see this topic"
      group_request_sent: "Your group membership request has been sent. You will be informed when it's accepted."
+      unread_indicator: "No member has read the last post of this topic yet."

      # keys ending with _MF use message format, see https://meta.discourse.org/t/message-format-support-for-localization/7035 for details
      read_more_MF: "There {
--- a/lib/svg_sprite/svg_sprite.rb
+++ b/lib/svg_sprite/svg_sprite.rb
@ -21,6 +21,7 @@ module SvgSprite
    "arrows-alt-h",
    "arrows-alt-v",
    "at",
+    "asterisk",
    "backward",
    "ban",
    "bars",
--- a/spec/models/topic_tracking_state_spec.rb
+++ b/spec/models/topic_tracking_state_spec.rb
@ -256,7 +256,7 @@ describe TopicTrackingState do

  describe '#publish_read_private_message' do
    fab!(:group) { Fabricate(:group) }
-    let(:read_topic_key) { "/private-messages/read-indicator/#{@group_message.id}" }
+    let(:read_topic_key) { "/private-messages/unread-indicator/#{@group_message.id}" }
    let(:read_post_key) { "/topic/#{@group_message.id}" }
    let(:latest_post_number) { 3 }

@ -281,15 +281,26 @@ describe TopicTrackingState do
    context 'when the read indicator is enabled' do
      before { group.update!(publish_read_state: true) }

-      it 'does publish the read indicator' do
+      it 'publishes a message to hide the unread indicator' do
        message = MessageBus.track_publish(read_topic_key) do
          TopicTrackingState.publish_read_indicator_on_read(@group_message.id, latest_post_number, user.id)
        end.first

        expect(message.data['topic_id']).to eq @group_message.id
+        expect(message.data['show_indicator']).to eq false
      end

-      it 'does not publish the read indicator if the message is not the last one' do
+      it 'publishes a message to show the unread indicator when a non-member creates a new post' do
+        allowed_user = Fabricate(:topic_allowed_user, topic: @group_message)
+        message = MessageBus.track_publish(read_topic_key) do
+          TopicTrackingState.publish_read_indicator_on_write(@group_message.id, latest_post_number, allowed_user.id)
+        end.first
+
+        expect(message.data['topic_id']).to eq @group_message.id
+        expect(message.data['show_indicator']).to eq true
+      end
+
+      it 'does not publish the unread indicator if the message is not the last one' do
        not_last_post_number = latest_post_number - 1
        Fabricate(:post, topic: @group_message, post_number: not_last_post_number)
        messages = MessageBus.track_publish(read_topic_key) do
--- a/vendor/assets/svg-icons/fontawesome/solid.svg
+++ b/vendor/assets/svg-icons/fontawesome/solid.svg
@ -3748,4 +3748,8 @@
    <title id="book-reader-title">Book Reader</title>
    <path d="M352 96c0-53.02-42.98-96-96-96s-96 42.98-96 96 42.98 96 96 96 96-42.98 96-96zM233.59 241.1c-59.33-36.32-155.43-46.3-203.79-49.05C13.55 191.13 0 203.51 0 219.14v222.8c0 14.33 11.59 26.28 26.49 27.05 43.66 2.29 131.99 10.68 193.04 41.43 9.37 4.72 20.48-1.71 20.48-11.87V252.56c-.01-4.67-2.32-8.95-6.42-11.46zm248.61-49.05c-48.35 2.74-144.46 12.73-203.78 49.05-4.1 2.51-6.41 6.96-6.41 11.63v245.79c0 10.19 11.14 16.63 20.54 11.9 61.04-30.72 149.32-39.11 192.97-41.4 14.9-.78 26.49-12.73 26.49-27.06V219.14c-.01-15.63-13.56-28.01-29.81-27.09z"></path>
  </symbol>
+  <symbol id="asterisk" viewBox="0 0 512 512">
+    <title id="asterisk-title">Asterisk</title>
+    <path d="M478.21 334.093L336 256l142.21-78.093c11.795-6.477 15.961-21.384 9.232-33.037l-19.48-33.741c-6.728-11.653-21.72-15.499-33.227-8.523L296 186.718l3.475-162.204C299.763 11.061 288.937 0 275.48 0h-38.96c-13.456 0-24.283 11.061-23.994 24.514L216 186.718 77.265 102.607c-11.506-6.976-26.499-3.13-33.227 8.523l-19.48 33.741c-6.728 11.653-2.562 26.56 9.233 33.037L176 256 33.79 334.093c-11.795 6.477-15.961 21.384-9.232 33.037l19.48 33.741c6.728 11.653 21.721 15.499 33.227 8.523L216 325.282l-3.475 162.204C212.237 500.939 223.064 512 236.52 512h38.961c13.456 0 24.283-11.061 23.995-24.514L296 325.282l138.735 84.111c11.506 6.976 26.499 3.13 33.227-8.523l19.48-33.741c6.728-11.653 2.563-26.559-9.232-33.036z"></path>
+  </symbol>
 </svg>