From 8db1f1892dd4d97b2a3cf44cb2f45e460e2a119d Mon Sep 17 00:00:00 2001
From: chapoi <101828855+chapoi@users.noreply.github.com>
Date: Mon, 19 Dec 2022 12:31:45 +0100
Subject: [PATCH] UX: Hashtag autocomplete styling (#19426)

* UX: added fadeout + hashtag styling

UX: add full name to autocomplete

UX: autocomplete mentions styling

UX: emoji styling user status

UX: autocomplete emoji

* DEV: Move hashtag tag counts into new secondary_text prop

* FIX: Add is-online style to mention users via chat

UX: make is-online avatar styling globally available

* DEV: Fix specs

* DEV: Test fix

Co-authored-by: Martin Brennan <martin@discourse.org>
---
 .../app/templates/hashtag-autocomplete.hbr    | 17 +++--
 .../templates/user-selector-autocomplete.hbr  |  5 +-
 .../stylesheets/common/base/compose.scss      | 67 ++++++++++++++-----
 .../common/components/hashtag.scss            | 67 +++++++++++++------
 app/services/hashtag_autocomplete_service.rb  |  3 +
 app/services/tag_hashtag_data_source.rb       | 13 ++--
 .../discourse/components/chat-composer.js     | 15 ++++-
 .../assets/stylesheets/common/common.scss     | 10 +++
 .../spec/system/hashtag_autocomplete_spec.rb  |  4 +-
 .../hashtag_autocomplete_service_spec.rb      | 29 ++++----
 spec/services/tag_hashtag_data_source_spec.rb |  6 +-
 spec/system/hashtag_autocomplete_spec.rb      | 10 +--
 12 files changed, 166 insertions(+), 80 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/templates/hashtag-autocomplete.hbr b/app/assets/javascripts/discourse/app/templates/hashtag-autocomplete.hbr
index 3c5979d51d3..49c3e488145 100644
--- a/app/assets/javascripts/discourse/app/templates/hashtag-autocomplete.hbr
+++ b/app/assets/javascripts/discourse/app/templates/hashtag-autocomplete.hbr
@@ -1,9 +1,12 @@
 <div class='autocomplete hashtag-autocomplete'>
-  <ul>
-    {{#each options as |option|}}
-      <li class="hashtag-autocomplete__option">
-        <a class="hashtag-autocomplete__link" title="{{option.description}}" href>{{d-icon option.icon}}<span class="hashtag-autocomplete__text">{{option.text}}</span></a>
-      </li>
-    {{/each}}
-  </ul>
+  <div class="hashtag-autocomplete__fadeout">
+    <ul>
+      {{#each options as |option|}}
+        <li class="hashtag-autocomplete__option">
+          <a class="hashtag-autocomplete__link" title="{{option.description}}" href>{{d-icon option.icon}}<span class="hashtag-autocomplete__text">{{option.text}}{{#if option.secondary_text}}<span class="hashtag-autocomplete__meta-text">({{option.secondary_text}}){{/if}}</span></span>
+          </a>
+        </li>
+      {{/each}}
+    </ul>
+  </div>
 </div>
diff --git a/app/assets/javascripts/discourse/app/templates/user-selector-autocomplete.hbr b/app/assets/javascripts/discourse/app/templates/user-selector-autocomplete.hbr
index 4b62706363e..90db6c3907d 100644
--- a/app/assets/javascripts/discourse/app/templates/user-selector-autocomplete.hbr
+++ b/app/assets/javascripts/discourse/app/templates/user-selector-autocomplete.hbr
@@ -2,9 +2,12 @@
   <ul>
     {{#each options.users as |user|}}
       <li>
-        <a href title="{{user.name}}">
+        <a href title="{{user.name}}" class="{{user.cssClasses}}">
           {{avatar user imageSize="tiny"}}
           <span class='username'>{{format-username user.username}}</span>
+          {{#if user.name}}
+            <span class='name'>{{user.name}}</span>
+          {{/if}}
           {{#if user.status}}
             {{emoji user.status.emoji}}
             <span class='status-description' title='{{user.status.description}}'>
diff --git a/app/assets/stylesheets/common/base/compose.scss b/app/assets/stylesheets/common/base/compose.scss
index ca76f74be64..f3a96cd973d 100644
--- a/app/assets/stylesheets/common/base/compose.scss
+++ b/app/assets/stylesheets/common/base/compose.scss
@@ -390,9 +390,12 @@ html.composer-open {
 .autocomplete {
   z-index: z("composer", "dropdown") + 1;
   position: absolute;
-  width: 240px;
+  max-width: 300px;
+  width: 300px;
   background-color: var(--secondary);
   border: 1px solid var(--primary-low);
+  box-shadow: shadow("dropdown-lite");
+  border-radius: 8px;
 
   ul {
     list-style: none;
@@ -400,16 +403,21 @@ html.composer-open {
     margin: 0;
 
     li {
-      &:not(:last-child) {
-        border-bottom: 1px solid var(--primary-low);
+      &:first-of-type a {
+        border-top-left-radius: 8px;
+        border-top-right-radius: 8px;
+      }
+      &:last-of-type a {
+        border-bottom-left-radius: 8px;
+        border-bottom-right-radius: 8px;
       }
-
       a {
         @include ellipsis;
         align-items: center;
-        color: var(--primary-high);
+        color: var(--primary);
         display: flex;
-        padding: 5px;
+        gap: 0.25em;
+        padding: 0.3em 1em;
 
         @include hover {
           background-color: var(--highlight-low);
@@ -417,20 +425,28 @@ html.composer-open {
         }
 
         &.selected {
-          background-color: var(--tertiary-low);
+          background-color: var(--highlight);
+
+          .username,
+          .name,
+          .emoji-shortname {
+            font-weight: bold;
+          }
         }
 
-        span {
-          margin-left: 5px;
+        .avatar {
+          margin-right: 0.25em;
+        }
 
-          &.username {
-            color: var(--primary);
-            margin-right: 5px;
-          }
+        .name {
+          font-size: var(--font-down-1);
+          color: var(--primary-high);
+        }
 
-          &.name {
-            font-size: var(--font-down-1);
-          }
+        .status-description {
+          @include ellipsis;
+          font-size: var(--font-down-2);
+          color: var(--primary-high);
         }
 
         .relative-date {
@@ -444,6 +460,25 @@ html.composer-open {
       }
     }
   }
+
+  &.ac-user {
+    li a {
+      padding: 0.5em 1em;
+    }
+    .emoji {
+      height: 0.75em;
+      width: 0.75em;
+    }
+  }
+
+  &.ac-emoji {
+    li:last-of-type a {
+      color: var(--primary-high);
+    }
+    .emoji {
+      margin-right: 0.25em;
+    }
+  }
 }
 
 div.ac-wrap.disabled {
diff --git a/app/assets/stylesheets/common/components/hashtag.scss b/app/assets/stylesheets/common/components/hashtag.scss
index 38876ec39ff..dbaccbbb123 100644
--- a/app/assets/stylesheets/common/components/hashtag.scss
+++ b/app/assets/stylesheets/common/components/hashtag.scss
@@ -36,31 +36,54 @@ a.hashtag-cooked {
 }
 
 .hashtag-autocomplete {
-  max-height: 210px;
+  max-height: 13.5em;
   overflow-y: auto;
+  box-shadow: shadow("dropdown-lite");
+  border-radius: 8px;
 
-  .hashtag-autocomplete__option {
-    .hashtag-autocomplete__link {
-      align-items: center;
-      color: var(--primary-medium);
-      display: flex;
+  &__fadeout {
+    height: inherit;
+    max-height: inherit;
+    overflow-y: auto;
+    -webkit-mask: linear-gradient(
+      180deg,
+      rgba(255, 255, 255, 1) calc(100% - 1.5em),
+      rgba(255, 255, 255, 0) 100%
+    );
+  }
 
-      .d-icon {
-        padding-right: 0.5em;
-      }
-
-      .hashtag-autocomplete__text {
-        flex: 1;
-        margin-left: 0;
-        text-overflow: ellipsis;
-        white-space: nowrap;
-        overflow: hidden;
-
-        .emoji {
-          width: 15px;
-          height: 15px;
-        }
-      }
+  &__option {
+    &:last-of-type {
+      margin-bottom: 0.75em; //used to the fadeout doesn't overlap the last item
     }
   }
+
+  &__link {
+    align-items: center;
+    display: flex;
+
+    &.selected {
+      font-weight: bold;
+    }
+
+    .d-icon {
+      color: var(--primary-medium);
+      margin-right: 0.25em;
+    }
+  }
+
+  &__text {
+    display: flex;
+    align-items: center;
+    gap: 0.25em;
+    flex: 1;
+    text-overflow: ellipsis;
+    white-space: nowrap;
+    overflow: hidden;
+  }
+
+  &__meta-text {
+    color: var(--primary-700);
+    font-size: var(--font-down-1);
+  }
 }
diff --git a/app/services/hashtag_autocomplete_service.rb b/app/services/hashtag_autocomplete_service.rb
index 0eceac75ce9..b13940382b5 100644
--- a/app/services/hashtag_autocomplete_service.rb
+++ b/app/services/hashtag_autocomplete_service.rb
@@ -62,6 +62,9 @@ class HashtagAutocompleteService
     # The text to display in the UI autocomplete menu for the item.
     attr_accessor :text
 
+    # Some items may want to display extra text in the UI styled differently, e.g. tag topic counts.
+    attr_accessor :secondary_text
+
     # The description text to display in the UI autocomplete menu on hover.
     # This will be things like e.g. category description.
     attr_accessor :description
diff --git a/app/services/tag_hashtag_data_source.rb b/app/services/tag_hashtag_data_source.rb
index dd5c6a0af96..67b445a8e8b 100644
--- a/app/services/tag_hashtag_data_source.rb
+++ b/app/services/tag_hashtag_data_source.rb
@@ -12,17 +12,14 @@ class TagHashtagDataSource
     "tag"
   end
 
-  def self.tag_to_hashtag_item(tag, include_count: false)
+  def self.tag_to_hashtag_item(tag)
     tag = Tag.new(tag.slice(:id, :name, :description).merge(topic_count: tag[:count])) if tag.is_a?(
       Hash,
     )
 
     HashtagAutocompleteService::HashtagItem.new.tap do |item|
-      if include_count
-        item.text = "#{tag.name} x #{tag.topic_count}"
-      else
-        item.text = tag.name
-      end
+      item.text = tag.name
+      item.secondary_text = "x#{tag.topic_count}"
       item.description = tag.description
       item.slug = tag.name
       item.relative_url = tag.url
@@ -66,7 +63,7 @@ class TagHashtagDataSource
     TagsController
       .tag_counts_json(tags_with_counts)
       .take(limit)
-      .map { |tag| tag_to_hashtag_item(tag, include_count: true) }
+      .map { |tag| tag_to_hashtag_item(tag) }
   end
 
   def self.search_sort(search_results, _)
@@ -89,6 +86,6 @@ class TagHashtagDataSource
     TagsController
       .tag_counts_json(tags_with_counts)
       .take(limit)
-      .map { |tag| tag_to_hashtag_item(tag, include_count: true) }
+      .map { |tag| tag_to_hashtag_item(tag) }
   end
 end
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js
index 1ce08f09700..932ca8f84f3 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js
@@ -395,7 +395,20 @@ export default Component.extend(TextareaTextManipulation, {
         treatAsTextarea: true,
         autoSelectFirstSuggestion: true,
         transformComplete: (v) => v.username || v.name,
-        dataSource: (term) => userSearch({ term, includeGroups: true }),
+        dataSource: (term) => {
+          return userSearch({ term, includeGroups: true }).then((result) => {
+            if (result?.users?.length > 0) {
+              const presentUserNames =
+                this.chat.presenceChannel.users?.mapBy("username");
+              result.users.forEach((user) => {
+                if (presentUserNames.includes(user.username)) {
+                  user.cssClasses = "mention-user-is-online";
+                }
+              });
+            }
+            return result;
+          });
+        },
         afterComplete: (text) => {
           this.set("value", text);
           this._focusTextArea();
diff --git a/plugins/chat/assets/stylesheets/common/common.scss b/plugins/chat/assets/stylesheets/common/common.scss
index 133a99b718e..0df0ebe0cd3 100644
--- a/plugins/chat/assets/stylesheets/common/common.scss
+++ b/plugins/chat/assets/stylesheets/common/common.scss
@@ -172,6 +172,16 @@ $float-height: 530px;
   }
 }
 
+.avatar {
+  border: 1px solid transparent;
+  padding: 0;
+
+  .is-online & {
+    border: 1px solid var(--secondary);
+    box-shadow: 0px 0px 0px 1px var(--success);
+  }
+}
+
 .chat-user-avatar {
   @include unselectable;
   display: flex;
diff --git a/plugins/chat/spec/system/hashtag_autocomplete_spec.rb b/plugins/chat/spec/system/hashtag_autocomplete_spec.rb
index 522a4e10009..a69c783b483 100644
--- a/plugins/chat/spec/system/hashtag_autocomplete_spec.rb
+++ b/plugins/chat/spec/system/hashtag_autocomplete_spec.rb
@@ -32,7 +32,7 @@ describe "Using #hashtag autocompletion to search for and lookup channels",
       count: 3,
     )
     hashtag_results = page.all(".hashtag-autocomplete__link", count: 3)
-    expect(hashtag_results.map(&:text)).to eq(["Random", "Raspberry", "razed x 0"])
+    expect(hashtag_results.map(&:text).map { |r| r.gsub("\n", " ") }).to eq(["Random", "Raspberry", "razed (x0)"])
   end
 
   it "searches for channels as well with # in a topic composer and deprioritises them" do
@@ -44,7 +44,7 @@ describe "Using #hashtag autocompletion to search for and lookup channels",
       count: 3,
     )
     hashtag_results = page.all(".hashtag-autocomplete__link", count: 3)
-    expect(hashtag_results.map(&:text)).to eq(["Raspberry", "razed x 0", "Random"])
+    expect(hashtag_results.map(&:text).map { |r| r.gsub("\n", " ") }).to eq(["Raspberry", "razed (x0)", "Random"])
   end
 
   it "cooks the hashtags for channels, categories, and tags serverside when the chat message is saved to the database" do
diff --git a/spec/services/hashtag_autocomplete_service_spec.rb b/spec/services/hashtag_autocomplete_service_spec.rb
index d1f591b3419..ffb10dacc41 100644
--- a/spec/services/hashtag_autocomplete_service_spec.rb
+++ b/spec/services/hashtag_autocomplete_service_spec.rb
@@ -39,19 +39,19 @@ RSpec.describe HashtagAutocompleteService do
   describe "#search" do
     it "returns search results for tags and categories by default" do
       expect(subject.search("book", %w[category tag]).map(&:text)).to eq(
-        ["The Book Club", "great-books x 22"],
+        ["The Book Club", "great-books"],
       )
     end
 
     it "respects the types_in_priority_order param" do
       expect(subject.search("book", %w[tag category]).map(&:text)).to eq(
-        ["great-books x 22", "The Book Club"],
+        ["great-books", "The Book Club"],
       )
     end
 
     it "respects the limit param" do
       expect(subject.search("book", %w[tag category], limit: 1).map(&:text)).to eq(
-        ["great-books x 22"],
+        ["great-books"],
       )
     end
 
@@ -72,16 +72,13 @@ RSpec.describe HashtagAutocompleteService do
     it "includes the tag count" do
       tag1.update!(topic_count: 78)
       expect(subject.search("book", %w[tag category]).map(&:text)).to eq(
-        ["great-books x 78", "The Book Club"],
+        ["great-books", "The Book Club"],
       )
     end
 
     it "does case-insensitive search" do
-      expect(subject.search("book", %w[category tag]).map(&:text)).to eq(
-        ["The Book Club", "great-books x 22"],
-      )
       expect(subject.search("bOOk", %w[category tag]).map(&:text)).to eq(
-        ["The Book Club", "great-books x 22"],
+        ["The Book Club", "great-books"],
       )
     end
 
@@ -92,7 +89,7 @@ RSpec.describe HashtagAutocompleteService do
 
     it "does not include categories the user cannot access" do
       category1.update!(read_restricted: true)
-      expect(subject.search("book", %w[tag category]).map(&:text)).to eq(["great-books x 22"])
+      expect(subject.search("book", %w[tag category]).map(&:text)).to eq(["great-books"])
     end
 
     it "does not include tags the user cannot access" do
@@ -111,7 +108,7 @@ RSpec.describe HashtagAutocompleteService do
       )
 
       expect(subject.search("book", %w[category tag bookmark]).map(&:text)).to eq(
-        ["The Book Club", "great-books x 22", "read review of this fantasy book"],
+        ["The Book Club", "great-books", "read review of this fantasy book"],
       )
     end
 
@@ -177,12 +174,12 @@ RSpec.describe HashtagAutocompleteService do
           "Horror",
           "Book Library",
           "Book Reviews",
-          "bookmania x 15",
+          "bookmania",
           "Abstract Philosophy",
           "Romance",
           "The Book Club",
-          "awful-books x 56",
-          "great-books x 22",
+          "awful-books",
+          "great-books",
         ],
       )
     end
@@ -246,9 +243,9 @@ RSpec.describe HashtagAutocompleteService do
             "Media",
             "Bookworld",
             Category.find(SiteSetting.uncategorized_category_id).name,
-            "mid-books x 33",
-            "great-books x 22",
-            "book x 1",
+            "mid-books",
+            "great-books",
+            "book",
           ],
         )
       end
diff --git a/spec/services/tag_hashtag_data_source_spec.rb b/spec/services/tag_hashtag_data_source_spec.rb
index cffe7ef097f..94a9dd412f3 100644
--- a/spec/services/tag_hashtag_data_source_spec.rb
+++ b/spec/services/tag_hashtag_data_source_spec.rb
@@ -31,9 +31,9 @@ RSpec.describe TagHashtagDataSource do
       )
     end
 
-    it "includes the topic count for the text of the tag" do
-      expect(described_class.search(guardian, "fact", 5).map(&:text)).to eq(
-        ["fact x 0", "factor x 5", "factory x 4", "factorio x 3", "factz x 1"],
+    it "includes the topic count for the text of the tag in secondary text" do
+      expect(described_class.search(guardian, "fact", 5).map(&:secondary_text)).to eq(
+        ["x0", "x5", "x4", "x3", "x1"],
       )
     end
 
diff --git a/spec/system/hashtag_autocomplete_spec.rb b/spec/system/hashtag_autocomplete_spec.rb
index 9dcc75874d0..4366a156487 100644
--- a/spec/system/hashtag_autocomplete_spec.rb
+++ b/spec/system/hashtag_autocomplete_spec.rb
@@ -35,19 +35,21 @@ describe "Using #hashtag autocompletion to search for and lookup categories and
   it "searches for categories and tags with # and prioritises categories in the results" do
     visit_topic_and_initiate_autocomplete
     hashtag_results = page.all(".hashtag-autocomplete__link", count: 2)
-    expect(hashtag_results.map(&:text)).to eq(["Cool Category", "cooltag x 325"])
+    expect(hashtag_results.map(&:text).map { |r| r.gsub("\n", " ") }).to eq(
+      ["Cool Category", "cooltag (x325)"],
+    )
   end
 
   it "begins showing results as soon as # is pressed based on categories and tags topic_count" do
     visit_topic_and_initiate_autocomplete(initiation_text: "#", expected_count: 5)
     hashtag_results = page.all(".hashtag-autocomplete__link")
-    expect(hashtag_results.map(&:text)).to eq(
+    expect(hashtag_results.map(&:text).map { |r| r.gsub("\n", " ") }).to eq(
       [
         "Cool Category",
         "Other Category",
         uncategorized_category.name,
-        "cooltag x 325",
-        "othertag x 66",
+        "cooltag (x325)",
+        "othertag (x66)",
       ],
     )
   end