fix: update topics listing UI (#317)

* fix: update topics listing UI * test: update archived topics test case * style: remove topic list last divider
2022-10-07 14:32:00 +05:00
parent 150f80412e
commit b5ed63c2ed
10 changed files with 225 additions and 98 deletions
--- a/src/discussions/navigation/breadcrumb-menu/BreadcrumbMenu.jsx
+++ b/src/discussions/navigation/breadcrumb-menu/BreadcrumbMenu.jsx
@@ -44,7 +44,7 @@ function BreadcrumbMenu() {
  });

  return (
-    <div className="breadcrumb-menu d-flex flex-row mt-2 mx-3">
+    <div className="breadcrumb-menu d-flex flex-row bg-light-200 box-shadow-down-1 px-2.5 py-1">
      <BreadcrumbDropdown
        currentItem={currentChapter}
        showAllPath={discussionsPath(Routes.TOPICS.ALL, { courseId })}
--- a/src/discussions/navigation/breadcrumb-menu/LegacyBreadcrumbMenu.jsx
+++ b/src/discussions/navigation/breadcrumb-menu/LegacyBreadcrumbMenu.jsx
@@ -31,7 +31,7 @@ function LegacyBreadcrumbMenu() {
  const isNonCoursewareTopic = currentTopic && !currentCategory;

  return (
-    <div className="breadcrumb-menu d-flex flex-row mt-2 mx-3">
+    <div className="breadcrumb-menu d-flex flex-row bg-light-200 box-shadow-down-1 px-2.5 py-1">
      {isNonCoursewareTopic ? (
        <BreadcrumbDropdown
          currentItem={currentTopic}
--- a/src/discussions/posts/PostsView.jsx
+++ b/src/discussions/posts/PostsView.jsx
@@ -74,9 +74,14 @@ function PostsView() {

  return (
    <div className="discussion-posts d-flex flex-column h-100">
-      {
-        searchString && <SearchInfo count={resultsFound} text={searchString} loadingStatus={loadingStatus} onClear={() => dispatch(setSearchQuery(''))} />
-      }
+      {searchString && (
+        <SearchInfo
+          count={resultsFound}
+          text={searchString}
+          loadingStatus={loadingStatus}
+          onClear={() => dispatch(setSearchQuery(''))}
+        />
+      )}
      <PostFilterBar />
      <div className="border-bottom border-light-400" />
      <div className="list-group list-group-flush flex-fill" role="list" onKeyDown={e => handleKeyDown(e)}>
--- a/src/discussions/posts/post/PostLink.jsx
+++ b/src/discussions/posts/post/PostLink.jsx
@@ -63,7 +63,7 @@ function PostLink({
        style={{ lineHeight: '21px' }}
        aria-current={isSelected(post.id) ? 'page' : undefined}
        role="option"
-        tabindex={(isSelected(post.id) || idx === 0) ? 0 : -1}
+        tabIndex={(isSelected(post.id) || idx === 0) ? 0 : -1}
      >
        <div
          className={
--- a/src/discussions/topics/TopicsView.jsx
+++ b/src/discussions/topics/TopicsView.jsx
@@ -22,15 +22,19 @@ function CourseWideTopics() {
  const { category } = useParams();
  const filter = useSelector(selectTopicFilter);
  const nonCoursewareTopics = useSelector(selectNonCoursewareTopics);
-  return (nonCoursewareTopics && category === undefined) && nonCoursewareTopics.filter(
-    item => (filter
-      ? item.name.toLowerCase()
-        .includes(filter)
-      : true
-    ),
-  )
-    .map(topic => (
-      <Topic topic={topic} key={topic.id} />
+  const filteredNonCoursewareTopics = nonCoursewareTopics.filter(item => (filter
+    ? item.name.toLowerCase().includes(filter)
+    : true
+  ));
+
+  return (nonCoursewareTopics && category === undefined)
+    && filteredNonCoursewareTopics.map((topic, index) => (
+      <Topic
+        topic={topic}
+        key={topic.id}
+        index={index}
+        showDivider={(filteredNonCoursewareTopics.length - 1) !== index}
+      />
    ));
 }

@@ -76,6 +80,20 @@ function TopicsView() {
  const { courseId } = useContext(DiscussionContext);
  const dispatch = useDispatch();

+  const handleKeyDown = (event) => {
+    const { key } = event;
+    if (key !== 'ArrowDown' && key !== 'ArrowUp') { return; }
+    const option = event.target;
+
+    let selectedOption;
+    if (key === 'ArrowDown') { selectedOption = option.nextElementSibling; }
+    if (key === 'ArrowUp') { selectedOption = option.previousElementSibling; }
+
+    if (selectedOption) {
+      selectedOption.focus();
+    }
+  };
+
  useEffect(() => {
    // Don't load till the provider information is available
    if (provider) {
@@ -89,19 +107,19 @@ function TopicsView() {
  }, [topicFilter]);

  return (
-    <div className="d-flex flex-column flex-fill">
-      <div
-        className="discussion-topics card"
-        data-testid="topics-view"
-      >
-        {
-          topicFilter && <SearchInfo text={topicFilter} count={filteredTopicsCount} loadingStatus={loadingStatus} onClear={() => dispatch(setFilter(''))} />
-        }
-        <div className="list-group list-group-flush">
-          <CourseWideTopics />
-          {provider === DiscussionProvider.OPEN_EDX && <CoursewareTopics />}
-          {provider === DiscussionProvider.LEGACY && <LegacyCoursewareTopics />}
-        </div>
+    <div className="discussion-topics d-flex flex-column h-100" data-testid="topics-view">
+      {topicFilter && (
+        <SearchInfo
+          text={topicFilter}
+          count={filteredTopicsCount}
+          loadingStatus={loadingStatus}
+          onClear={() => dispatch(setFilter(''))}
+        />
+      )}
+      <div className="list-group list-group-flush flex-fill" role="list" onKeyDown={e => handleKeyDown(e)}>
+        <CourseWideTopics />
+        {provider === DiscussionProvider.OPEN_EDX && <CoursewareTopics />}
+        {provider === DiscussionProvider.LEGACY && <LegacyCoursewareTopics />}
      </div>
      {
        filteredTopicsCount === 0
--- a/src/discussions/topics/TopicsView.test.jsx
+++ b/src/discussions/topics/TopicsView.test.jsx
@@ -159,7 +159,7 @@ describe('TopicsView', () => {
        renderComponent();
        const archivedTopicGroup = screen.queryAllByTestId('topic-group').pop();
        expect(archivedTopicGroup).toHaveTextContent(/archived/i);
-        const archivedTopicLinks = within(archivedTopicGroup).queryAllByRole('link');
+        const archivedTopicLinks = within(archivedTopicGroup).queryAllByRole('option');
        expect(archivedTopicLinks).toHaveLength(2);
      });
    }
--- a/src/discussions/topics/messages.js
+++ b/src/discussions/topics/messages.js
@@ -1,6 +1,34 @@
 import { defineMessages } from '@edx/frontend-platform/i18n';

 const messages = defineMessages({
+  discussions: {
+    id: 'discussions.topics.discussions',
+    defaultMessage: `{count, plural,
+      =0 {Discussion}
+      one {# Discussion}
+      other {# Discussions}
+    }`,
+    description: 'Display tooltip text used to indicate how many posts type are discussion',
+  },
+  questions: {
+    id: 'discussions.topics.questions',
+    defaultMessage: `{count, plural,
+      =0 {Question}
+      one {# Question}
+      other {# Questions}
+    }`,
+    description: 'Display tooltip text used to indicate how many posts type are questions',
+  },
+  reported: {
+    id: 'discussions.topics.reported',
+    defaultMessage: '{reported} reported',
+    description: 'Display tooltip text used to indicate how many posts are reported',
+  },
+  previouslyReported: {
+    id: 'discussions.topics.previouslyReported',
+    defaultMessage: '{previouslyReported} previously reported',
+    description: 'Display tooltip text used to indicate how many posts are previously reported',
+  },
  sortedBy: {
    id: 'discussions.topics.sort.message',
    defaultMessage: 'Sorted by {sortBy}',
--- a/src/discussions/topics/topic-group/TopicGroupBase.jsx
+++ b/src/discussions/topics/topic-group/TopicGroupBase.jsx
@@ -24,45 +24,50 @@ function TopicGroupBase({
  const filter = useSelector(selectTopicFilter);
  const hasTopics = topics.length > 0;
  const matchesFilter = filter
-    ? groupTitle?.toLowerCase()
-      .includes(filter)
+    ? groupTitle?.toLowerCase().includes(filter)
    : true;
-  const topicElements = topics.filter(
-    topic => (
-      filter
-        ? (topic.name.toLowerCase()
-          .includes(filter) || matchesFilter)
-        : true
+
+  const filteredTopicElements = topics.filter(
+    topic => (filter
+      ? (topic.name.toLowerCase().includes(filter) || matchesFilter)
+      : true
    ),
-  )
-    .map(topic => (<Topic topic={topic} key={topic.id} />));
-  const hasFilteredSubtopics = (topicElements.length > 0);
+  );
+
+  const hasFilteredSubtopics = (filteredTopicElements.length > 0);
  if (!hasTopics || (!matchesFilter && !hasFilteredSubtopics)) {
    return null;
  }
  return (
    <div
-      className="discussion-topic-group d-flex flex-column"
+      className="discussion-topic-group d-flex flex-column text-primary-500"
      data-category-id={groupId}
      data-testid="topic-group"
    >
-      {linkToGroup && groupId
-        ? (
-          <Link
-            className="list-group-item p-4 text-primary-500"
-            to={discussionsPath(Routes.TOPICS.CATEGORY, {
-              courseId,
-              category: groupId,
-            })}
-          >
-            {groupTitle}
-          </Link>
-        ) : (
-          <span className="list-group-item p-4 text-primary-500">
-            {groupTitle || intl.formatMessage(messages.unnamedTopicCategories)}
-          </span>
-        )}
-      {topicElements}
+      <div className="pt-2.5 px-4 font-weight-bold">
+        {linkToGroup && groupId
+          ? (
+            <Link
+              className="text-decoration-none text-primary-500"
+              to={discussionsPath(Routes.TOPICS.CATEGORY, {
+                courseId,
+                category: groupId,
+              })}
+            >
+              {groupTitle}
+            </Link>
+          ) : (
+            groupTitle || intl.formatMessage(messages.unnamedTopicCategories)
+          )}
+      </div>
+      {filteredTopicElements.map((topic, index) => (
+        <Topic
+          topic={topic}
+          key={topic.id}
+          index={index}
+          showDivider={(filteredTopicElements.length - 1) !== index}
+        />
+      ))}
    </div>
  );
 }
--- a/src/discussions/topics/topic-group/topic/Topic.jsx
+++ b/src/discussions/topics/topic-group/topic/Topic.jsx
@@ -2,65 +2,120 @@
 import React from 'react';
 import PropTypes from 'prop-types';

+import classNames from 'classnames';
+import { useSelector } from 'react-redux';
 import { useParams } from 'react-router';
 import { Link } from 'react-router-dom';

-import { Icon } from '@edx/paragon';
-import { Error as ErrorIcon, Help, PostOutline } from '@edx/paragon/icons';
+import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
+import { Icon, OverlayTrigger, Tooltip } from '@edx/paragon';
+import { HelpOutline, PostOutline, Report } from '@edx/paragon/icons';

 import { Routes } from '../../../../data/constants';
+import { selectUserHasModerationPrivileges, selectUserIsGroupTa } from '../../../data/selectors';
 import { discussionsPath } from '../../../utils';
+import messages from '../../messages';

-function Topic({ topic }) {
+function Topic({
+  topic,
+  showDivider,
+  index,
+  intl,
+}) {
  const { courseId } = useParams();
  const topicUrl = discussionsPath(Routes.TOPICS.TOPIC, {
    courseId,
    topicId: topic.id,
  });
-  const icons = [
-    {
-      key: 'discussions',
-      icon: PostOutline,
-      count: topic.threadCounts?.discussion || 0,
-    },
-    {
-      key: 'questions',
-      icon: Help,
-      count: topic.threadCounts?.question || 0,
-    },
-  ];
+  const userHasModerationPrivileges = useSelector(selectUserHasModerationPrivileges);
+  const userIsGroupTa = useSelector(selectUserIsGroupTa);
+  const { inactiveFlags, activeFlags } = topic;
+  const canSeeReportedStats = (activeFlags || inactiveFlags) && (userHasModerationPrivileges || userIsGroupTa);
+  const isSelected = (id) => window.location.pathname.includes(id);
+
  return (
    <Link
-      className="discussion-topic d-flex flex-column list-group-item px-4 py-3 text-primary-500"
+      className={
+        classNames('discussion-topic p-0 text-decoration-none text-primary-500', {
+          'border-bottom border-light-400': showDivider,
+        })
+      }
      data-topic-id={topic.id}
      to={topicUrl}
+      onClick={() => isSelected(topic.id)}
+      aria-current={isSelected(topic.id) ? 'page' : undefined}
+      role="option"
+      tabIndex={(isSelected(topic.id) || index === 0) ? 0 : -1}
    >
-      <div className="topic-name">
-        {topic.name}
-      </div>
-      <div className="d-flex mt-3">
-        {
-          icons.map(({
-            key,
-            icon,
-            count,
-          }) => (
-            <div className="mr-4 d-flex align-items-center" key={key}>
-              <Icon className="mr-2" src={icon} />
-              {/* Reserve some space for larger counts */}
-              <span>
-                {count}
-              </span>
+      <div className="d-flex flex-row pt-2.5 pb-2 px-4">
+        <div className="d-flex flex-column flex-fill" style={{ minWidth: 0 }}>
+          <div className="d-flex flex-column justify-content-start mw-100 flex-fill">
+            <div className="topic-name text-truncate">
+              {topic.name}
            </div>
-          ))
-        }
-        {topic?.flags && (
-          <div className="d-flex align-items-center">
-            <Icon className="mr-2" src={ErrorIcon} />
-            {topic.flags}
          </div>
-        )}
+          <div className="d-flex align-items-center mt-2.5" style={{ marginBottom: '2px' }}>
+            <OverlayTrigger
+              overlay={(
+                <Tooltip>
+                  <div className="d-flex flex-column align-items-start">
+                    {intl.formatMessage(messages.discussions, {
+                      count: topic.threadCounts?.discussion || 0,
+                    })}
+                  </div>
+                </Tooltip>
+                )}
+            >
+              <div className="d-flex align-items-center mr-3.5">
+                <Icon src={PostOutline} className="icon-size mr-2" />
+                {topic.threadCounts?.discussion || 0}
+              </div>
+            </OverlayTrigger>
+            <OverlayTrigger
+              overlay={(
+                <Tooltip>
+                  <div className="d-flex flex-column align-items-start">
+                    {intl.formatMessage(messages.questions, {
+                      count: topic.threadCounts?.question || 0,
+                    })}
+                  </div>
+                </Tooltip>
+              )}
+            >
+              <div className="d-flex align-items-center mr-3.5">
+                <Icon src={HelpOutline} className="icon-size mr-2" />
+                {topic.threadCounts?.question || 0}
+              </div>
+            </OverlayTrigger>
+            {Boolean(canSeeReportedStats) && (
+              <OverlayTrigger
+                overlay={(
+                  <Tooltip>
+                    <div className="d-flex flex-column align-items-start">
+                      {Boolean(activeFlags) && (
+                        <span>
+                          {intl.formatMessage(messages.reported, { reported: activeFlags })}
+                        </span>
+                      )}
+                      {Boolean(inactiveFlags) && (
+                        <span>
+                          {intl.formatMessage(messages.previouslyReported, { previouslyReported: inactiveFlags })}
+                        </span>
+                      )}
+                    </div>
+                  </Tooltip>
+                )}
+              >
+                <div className="d-flex align-items-center">
+                  <Icon src={Report} className="icon-size mr-2 text-danger" />
+                  {activeFlags}{Boolean(inactiveFlags) && `/${inactiveFlags}`}
+                </div>
+              </OverlayTrigger>
+            )}
+          </div>
+        </div>
      </div>
+      {!showDivider && <div className="divider pt-1 bg-light-500 border-top border-light-700" />}
    </Link>
  );
 }
@@ -73,7 +128,15 @@ export const topicShape = PropTypes.shape({
  flags: PropTypes.number,
 });
 Topic.propTypes = {
+  intl: intlShape.isRequired,
  topic: topicShape.isRequired,
+  showDivider: PropTypes.bool,
+  index: PropTypes.number,
 };

-export default Topic;
+Topic.defaultProps = {
+  showDivider: true,
+  index: -1,
+};
+
+export default injectIntl(Topic);
--- a/src/index.scss
+++ b/src/index.scss
@@ -232,3 +232,11 @@ header {
  position: sticky;
  top: 0;
 }
+
+.breadcrumb-menu {
+  z-index: 1;
+}
+
+.discussion-topic-group:last-of-type .divider{
+  display: none;
+}