Asking for feedback: Tag Reveal Component in Topic Lists - Expand/Collapse tags in topic lists

Development
19

I see. I think it makes sense if you display information as a tag, but here, it’s a button to display more tags; the context is different to me. It’s up to you; I don’t think it matters that much.

To continue with the feedback:

  • The tags list can be displayed in other places, such as: categories page, user’s activities, etc. I would probably remove collapse_in_topic_view setting and make either a new one with specific routes or just enable it everywhere.
    In my test code, I used something like that to ignore other routes:

    JS 
      function isAllowedRoute(routeName) {
      const fullRoutesName = [
        "index",
        "userActivity.topics",
        "userActivity.read",
        ...siteSettings.top_menu.split("|").map((item) => `discovery.${item}`),
      ];
  
      const partialRoutesName = ["topic."];
  
      if (
        fullRoutesName.includes(routeName) ||
        partialRoutesName.some((partial) => routeName.startsWith(partial))
      ) {
        return true;
      }
  
      return false;
    }

  • The CSS injection can be replaced using the API to add a class to topic-list-item and to a tag, then you move the CSS to common.css.

    For example:

    JS 
    import { defaultRenderTag } from "discourse/lib/render-tag";

api.registerValueTransformer(
  "topic-list-item-class",
  ({ value, context }) => {
    if (highlightedTagsSet.size === 0) {
      return value;
    }

    if (context.topic?.tags?.some((tag) => highlightedTagsSet.has(tag))) {
      return [...value, `highlighted-tag__${settings.highlighted_style}`];
    }

    return value;
  }
);

api.replaceTagRenderer((tag, params) => {
  if (highlightedTagsSet.has(tag)) {
    params.extraClass = params.extraClass || "";
    params.extraClass += "highlighted";
  }

  return defaultRenderTag(tag, params);
});
    CSS 
    /* Hides the last separator before the toggle button */
.discourse-tags__tag-separator:has(+ .reveal-tag-action) {
  visibility: hidden;
}

.reveal-tag-action {
  color: var(--primary-500);

  &.-box {
    background-color: var(--primary-50);
    outline: 1px solid var(--primary-200);
    padding-inline: 8px;
  }
}

.latest-topic-list-item,
.topic-list-item {
  .discourse-tag.highlighted {
    color: var(--tertiary);
    border-color: var(--tertiary);
    background: color-mix(in srgb, var(--tertiary) 12%, transparent);
    font-weight: 600;
  }

  &.highlighted-tag {
    &__left-border {
      border-left: 3px solid var(--tertiary);
      background: color-mix(in srgb, var(--tertiary) 6%, transparent);
      transition: box-shadow 160ms ease;

      &:hover {
        box-shadow: 0 2px 10px rgba(0, 0, 0, 0.08);
      }
    }

    &__outline {
      outline: 1px solid var(--tertiary);
      outline-offset: -2px;
      border-radius: 7px;
      background: color-mix(in srgb, var(--tertiary) 5%, transparent);
      box-shadow: 0 1px 6px rgba(0, 0, 0, 0.06);
      transition: background-color 160ms ease;
    }

    &__card {
      border-left: 3px solid var(--tertiary);
      background: var(--tertiary-very-low);
      border-radius: var(--border-radius);
      padding-block: var(--space-2);
      box-shadow: 0 1px 4px rgba(0, 0, 0, 0.06);
      transition: box-shadow 160ms ease;

      &:hover {
        box-shadow: 0 3px 12px rgba(0, 0, 0, 0.1);
      }
    }
  }
}

  • You don’t need to set the current route from onPageChange, you can access it from the router.

  • Be careful with the tag letter case. You have site settings that do not enforce lowercase, so I think it’s best not to modify the tag.

  • About resetting the state, you can probably use onPageChange.

    JS 
    api.onPageChange((url) => {
    const route = api.container.lookup("service:router").recognize(url);
    if (!isAllowedRoute(route?.name)) {
      return;
    }

    for (const [id, model] of topicModels) {
      if (model && model.revealTags) {
        model.revealTags = false;
        model.notifyPropertyChange("tags");
      }
    }
  });

  • If you can, it would be great to add tests.

Here is the full test code (I did other minor changes)

JS 
import { computed } from "@ember/object";
import { apiInitializer } from "discourse/lib/api";
import { i18n } from "discourse-i18n";
import { defaultRenderTag } from "discourse/lib/render-tag";
import { service } from "@ember/service";

export default apiInitializer((api) => {
  const siteSettings = api.container.lookup("service:site-settings");
  const router = api.container.lookup("service:router");

  const maxVisibleTags = Math.min(
    settings.max_tags_visible,
    siteSettings.max_tags_per_topic
  );

  const highlightedTagsSet = new Set(settings.highlighted_tags.split("|"));
  const topicModels = new Map();

  function isAllowedRoute(routeName) {
    const fullRoutesName = [
      "index",
      "userActivity.topics",
      "userActivity.read",
      "tag.show",
      ...siteSettings.top_menu.split("|").map((item) => `discovery.${item}`),
    ];

    const partialRoutesName = ["topic."];

    if (
      fullRoutesName.includes(routeName) ||
      partialRoutesName.some((partial) => routeName.startsWith(partial))
    ) {
      return true;
    }

    return false;
  }

  api.modifyClass(
    "model:topic",
    (Superclass) =>
      class extends Superclass {
        @service router;

        revealTags = false;

        init() {
          super.init(...arguments);
          topicModels.set(String(this.id), this);
        }

        willDestroy() {
          super.willDestroy(...arguments);
          topicModels.delete(String(this.id));
        }

        @computed("tags")
        get visibleListTags() {
          const baseTags = super.visibleListTags || [];

          if (!isAllowedRoute(this.router.currentRouteName)) {
            return baseTags;
          }

          const highlightedList = [];
          const regularList = [];

          baseTags.forEach((tag) => {
            if (highlightedTagsSet.has(tag)) {
              highlightedList.push(tag);
            } else {
              regularList.push(tag);
            }
          });

          if (this.revealTags) {
            return [...highlightedList, ...regularList];
          }

          return [...highlightedList, ...regularList.slice(0, maxVisibleTags)];
        }
      }
  );

  api.addTagsHtmlCallback(
    (topic) => {
      if (!isAllowedRoute(topic.router.currentRouteName)) {
        return "";
      }

      const allTags = topic.tags || [];
      if (allTags.length === 0) {
        return "";
      }

      const highlightedCount = allTags.filter((tag) =>
        highlightedTagsSet.has(tag)
      ).length;
      const regularCount = allTags.length - highlightedCount;
      const effectiveLimit =
        highlightedCount + Math.min(regularCount, maxVisibleTags);

      // Only show toggle if there are hidden tags
      if (allTags.length <= effectiveLimit) {
        return "";
      }

      const isExpanded = topic.revealTags;
      const hiddenCount = allTags.length - effectiveLimit;
      const label = isExpanded
        ? i18n(themePrefix("js.tag_reveal.hide"))
        : i18n(themePrefix("js.tag_reveal.more_tags"), {
            count: hiddenCount,
          });

      const classList = ["discourse-tag", "reveal-tag-action"];
      if (settings.toggle_tag_style === "box") {
        classList.push("-box");
      }

      return `<a class="${classList.join(" ")}" role="button" aria-expanded="${isExpanded}">${label}</a>`;
    },
    {
      priority: siteSettings.max_tags_per_topic + 1,
    }
  );

  api.registerValueTransformer(
    "topic-list-item-class",
    ({ value, context }) => {
      if (highlightedTagsSet.size === 0) {
        return value;
      }

      if (context.topic?.tags?.some((tag) => highlightedTagsSet.has(tag))) {
        return [...value, `highlighted-tag__${settings.highlighted_style}`];
      }

      return value;
    }
  );

  api.replaceTagRenderer((tag, params) => {
    let newParams = params;

    if (highlightedTagsSet.has(tag)) {
      newParams = {
        ...params,
        extraClass: [params.extraClass, "highlighted"]
          .filter(Boolean)
          .join(" "),
      };
    }

    return defaultRenderTag(tag, newParams);
  });

  document.addEventListener(
    "click",
    (event) => {
      const target = event.target;
      if (!target?.matches(".reveal-tag-action")) {
        return;
      }

      event.preventDefault();
      event.stopPropagation();

      const element =
        target.closest("[data-topic-id]") ||
        document.querySelector("h1[data-topic-id]");
      const topicId = element?.dataset.topicId;
      if (!topicId) {
        return;
      }

      const topicModel = topicModels.get(topicId);
      if (!topicModel) {
        return;
      }

      topicModel.revealTags = !topicModel.revealTags;
      topicModel.notifyPropertyChange("tags");
    },
    true
  );

  api.onPageChange((url) => {
    const route = api.container.lookup("service:router").recognize(url);
    if (!isAllowedRoute(route?.name)) {
      return;
    }

    for (const [id, model] of topicModels) {
      if (model && model.revealTags) {
        model.revealTags = false;
        model.notifyPropertyChange("tags");
      }
    }
  });
});